On Jan 27, 2021, at 01:38, Borislav Petkov [off-list ref] wrote:

On Wed, Jan 27, 2021 at 01:23:35AM +0000, Bae, Chang Seok wrote:

quoted

Okay. I will prepare a separate cleanup patch that can be applied at the end
of the series. Will post the change in this thread at first.

No, this is not how this works. Imagine you pile up a patch at the end
for each review feedback you've gotten. No, this will be an insane churn
and an unreviewable mess.

What you do is you rework your patches like everyone else.

Yeah, it makes sense. I will post v4.

Also, thinking about this more, I'm wondering if all those
xstate-related attributes shouldn't be part of struct fpu instead of
being scattered around like that.

That thing - struct fpu * - gets passed in everywhere anyway so all that
min_size, max_size, ->xstate_ptr and whatever, looks like it wants to be
part of struct fpu. Then maybe you won't need the accessors...

Well, min_size and max_size are not task-specific. So, it will be wasteful to
include in struct fpu.

I will follow your suggestion to add new helpers to access the size values,
instead of exporting them.

quoted

quoted

quoted

@@ -627,13 +627,18 @@ static void check_xstate_against_struct(int nr)

*/

<-- There's a comment over this function that might need adjustment.

Do you mean an empty line? (Just want to clarify.)

No, I mean this comment:

* Dynamic XSAVE features allocate their own buffers and are not
* covered by these checks. Only the size of the buffer for task->fpu
* is checked here.

That probably needs adjusting as you do set min and max size here now
for the dynamic buffer.

Oh, I see. Thank you.

quoted

Agreed. I will prepare a patch. At least will post the diff here.

You can send it separately from this patchset, ontop of current
tip/master, so that I can take it now.

Posted, [1]. After all, the proposal is to remove the helper.

Thanks,
Chang

[1] https://lore.kernel.org/lkml/20210203024052.15789-1-chang.seok.bae@intel.com/ (local)