Re: [PATCH overlayfs-next v3 0/4] overlayfs: Optimize override/revert creds

[Vinicius Costa Gomes <vinicius.gomes@intel.com>]

Hi,

Amir Goldstein <amir73il@gmail.com> writes:

> On Tue, Nov 5, 2024 at 8:35 PM Vinicius Costa Gomes
> <vinicius.gomes@intel.com> wrote:
>>
>> Hi,
>>
>> This series is rebased on top of Amir's overlayfs-next branch.
>>
>> Changes from v2:
>>  - Removed the "convert to guard()/scoped_guard()" patches (Miklos Szeredi);
>>  - In the overlayfs code, convert all users of override_creds()/revert_creds() to the _light() versions by:
>>       1. making ovl_override_creds() use override_creds_light();
>>       2. introduce ovl_revert_creds() which calls revert_creds_light();
>>       3. convert revert_creds() to ovl_revert_creds()
>>    (Amir Goldstein);
>>  - Fix an potential reference counting issue, as the lifetime
>>    expectations of the mounter credentials are different (Christian
>>    Brauner);
>>
>
> Hi Vicius,
>
> The end result looks good to me, but we still need to do the series a
> bit differently.
>
>> The series is now much simpler:
>>
>> Patch 1: Introduce the _light() version of the override/revert cred operations;
>> Patch 2: Convert backing-file.c to use those;
>> Patch 3: Do the conversion to use the _light() version internally;
>
> This patch mixes a small logic change and a large mechanical change
> that is not a good mix.
>
> I took the liberty to split out the large mechanical change to
> ovl: use wrapper ovl_revert_creds()
> and pushed it to branch
> https://github.com/amir73il/linux/commits/ovl_creds
>
> I then rebased overlayfs-next over this commit and resolved the
> conflicts with the pure mechanical change.
>
> Now you can rebase your patches over ovl_creds and they should
> not be conflicting with overlayfs-next changes.
>
> The reason I wanted to do this is that Christian could take your changes
> as well as my ovl_creds branch through the vfs tree if he chooses to do so.
>

Makes sense.

>> Patch 4: Fix a potential refcounting issue
>
> This patch cannot be separated from patch #3 because it would introduce the
> refcount leak mid series.
>
> But after I took out all the mechanical changes out of patch #3,
> there should be no problem for you to squash patches #3 and #4 together.
>

Done. 

> One more nit: please use "ovl: ..." for commit titles instead of
> "fs/overlayfs: ...".
>

Also done. Will give the series a round of testing, just to be sure, and
will send the next version tomorrow.

> Thanks,
> Amir.


Cheers,
-- 
Vinicius