Fwd: [PATCH 09/14] cifs: add a back pointer to cifs_sb from tcon

[Steve French]

resending to list

---------- Forwarded message ---------
From: Steve French <smfrench at gmail.com>
Date: Mon, Nov 6, 2023 at 1:13 PM
Subject: Re: [PATCH 09/14] cifs: add a back pointer to cifs_sb from tcon
To: Shyam Prasad N <nspmangalore at gmail.com>
Cc: Paulo Alcantara <pc at manguebit.com>, <bharathsm.hsk at gmail.com>,
<linux-cifs at vger.kernel.org>, Shyam Prasad N <sprasad at microsoft.com>


Updated patches again to address compile/sparse warning and pushed to
for-next pending more testing and review

Attached are updated patches 1 and 2

On Mon, Nov 6, 2023 at 12:45 PM Steve French <smfrench at gmail.com> wrote:
>
> lightly updated patch 1 and patch 2 in your series to fix checkpatch warnings
>
> Was thinking about patch 2 though and whether it could hurt cases where there is little parallelism - we could randomly pick channels to send things on so we could overuse channels with higher latency rather than preferring the first two channels which are the "faster" ones (and presumably lower latencies).   Was wondering if we end up picking e.g. 4 channels, 1 of which is "fast" and 3 are slower - we could end up with cases where no traffic on channel 1 but we end up sending on a longer latency channlel - ie the round robin approach could end up using channel 2 or 3 or 4 which have longer latencies even if no traffic on the "fastest" channel ...
>
>
> On Mon, Nov 6, 2023 at 11:04 AM Shyam Prasad N <nspmangalore at gmail.com> wrote:
>>
>> On Mon, Nov 6, 2023 at 9:42 PM Shyam Prasad N <nspmangalore at gmail.com> wrote:
>> >
>> > On Sat, Nov 4, 2023 at 2:33 AM Paulo Alcantara <pc at manguebit.com> wrote:
>> > >
>> > > nspmangalore at gmail.com writes:
>> > >
>> > > > From: Shyam Prasad N <sprasad at microsoft.com>
>> > > >
>> > > > Today, we have no way to access the cifs_sb when we
>> > > > just have pointers to struct tcon. This is very
>> > > > limiting as many functions deal with cifs_sb, and
>> > > > these calls do not directly originate from VFS.
>> > > >
>> > > > This change introduces a new cifs_sb field in cifs_tcon
>> > > > that points to the cifs_sb for the tcon. The assumption
>> > > > here is that a tcon will always map to this cifs_sb and
>> > > > will never change.
>> > > >
>> > > > Also, refcounting should not be necessary, since cifs_sb
>> > > > will never be freed before tcon.
>> > > >
>> > > > Signed-off-by: Shyam Prasad N <sprasad at microsoft.com>
>> > > > ---
>> > > >  fs/smb/client/cifsglob.h | 1 +
>> > > >  fs/smb/client/connect.c  | 2 ++
>> > > >  2 files changed, 3 insertions(+)
>> > >
>> > > This is wrong as a single tcon may be shared among different
>> > > superblocks.  You can, however, map those superblocks to a tcon by using
>> > > the cifs_sb_master_tcon() helper.
>> > >
>> > > If you do something like this
>> > >
>> > >         mount.cifs //srv/share /mnt/1 -o ...
>> > >         mount.cifs //srv/share /mnt/1 -o ... -> -EBUSY
>> > >
>> > > tcon->cifs_sb will end up with the already freed superblock pointer that
>> > > was compared to the existing one.  So, you'll get an use-after-free when
>> > > you dereference tcon->cifs_sb as in patch 11/14.
>> >
>> > Hi Steve,
>> > I discussed this one with Paulo separately. You can drop this patch.
>> > I'll have another patch in place of this one. And then send you an
>> > updated patch for the patch which depends on it.
>> >
>> > --
>> > Regards,
>> > Shyam
>>
>> Here's the replacement patch for "cifs: add a back pointer to cifs_sb from tcon"
>>
>> --
>> Regards,
>> Shyam
>
>
>
> --
> Thanks,
>
> Steve



-- 
Thanks,

Steve


-- 
Thanks,

Steve
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-cifs-distribute-channels-across-interfaces-based-on-.patch
Type: application/x-patch
Size: 7981 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20231106/6b82f538/0002-cifs-distribute-channels-across-interfaces-based-on-.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-cifs-handle-cases-where-a-channel-is-closed.patch
Type: application/x-patch
Size: 7593 bytes
Desc: not available
URL: <http://lists.samba.org/pipermail/samba-technical/attachments/20231106/6b82f538/0001-cifs-handle-cases-where-a-channel-is-closed.bin>