* [PATCH] smb: enable reuse of deferred file handles for write operations @ 2024-12-16 18:31 Bharath SM 2024-12-16 20:43 ` Steve French 0 siblings, 1 reply; 6+ messages in thread From: Bharath SM @ 2024-12-16 18:31 UTC (permalink / raw) To: linux-cifs, sfrench, pc, sprasad, tom, ronniesahlberg; +Cc: Bharath SM Previously, deferred file handles were reused only for read operations, this commit extends to reusing deferred handles for write operations. Signed-off-by: Bharath SM <bharathsm@microsoft.com> --- fs/smb/client/file.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c index a58a3333ecc3..98deff1de74c 100644 --- a/fs/smb/client/file.c +++ b/fs/smb/client/file.c @@ -990,7 +990,11 @@ int cifs_open(struct inode *inode, struct file *file) } /* Get the cached handle as SMB2 close is deferred */ - rc = cifs_get_readable_path(tcon, full_path, &cfile); + if (OPEN_FMODE(file->f_flags) & FMODE_WRITE) { + rc = cifs_get_writable_path(tcon, full_path, FIND_WR_ANY, &cfile); + } else { + rc = cifs_get_readable_path(tcon, full_path, &cfile); + } if (rc == 0) { if (file->f_flags == cfile->f_flags) { file->private_data = cfile; -- 2.43.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] smb: enable reuse of deferred file handles for write operations 2024-12-16 18:31 [PATCH] smb: enable reuse of deferred file handles for write operations Bharath SM @ 2024-12-16 20:43 ` Steve French 2024-12-19 9:33 ` Shyam Prasad N 0 siblings, 1 reply; 6+ messages in thread From: Steve French @ 2024-12-16 20:43 UTC (permalink / raw) To: Bharath SM Cc: linux-cifs, sfrench, pc, sprasad, tom, ronniesahlberg, Bharath SM merged into cifs-2.6.git for-next pending review and more testing On Mon, Dec 16, 2024 at 12:36 PM Bharath SM <bharathsm.hsk@gmail.com> wrote: > > Previously, deferred file handles were reused only for read > operations, this commit extends to reusing deferred handles > for write operations. > > Signed-off-by: Bharath SM <bharathsm@microsoft.com> > --- > fs/smb/client/file.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c > index a58a3333ecc3..98deff1de74c 100644 > --- a/fs/smb/client/file.c > +++ b/fs/smb/client/file.c > @@ -990,7 +990,11 @@ int cifs_open(struct inode *inode, struct file *file) > } > > /* Get the cached handle as SMB2 close is deferred */ > - rc = cifs_get_readable_path(tcon, full_path, &cfile); > + if (OPEN_FMODE(file->f_flags) & FMODE_WRITE) { > + rc = cifs_get_writable_path(tcon, full_path, FIND_WR_ANY, &cfile); > + } else { > + rc = cifs_get_readable_path(tcon, full_path, &cfile); > + } > if (rc == 0) { > if (file->f_flags == cfile->f_flags) { > file->private_data = cfile; > -- > 2.43.0 > > -- Thanks, Steve ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] smb: enable reuse of deferred file handles for write operations 2024-12-16 20:43 ` Steve French @ 2024-12-19 9:33 ` Shyam Prasad N 2024-12-19 22:47 ` Paul Aurich 0 siblings, 1 reply; 6+ messages in thread From: Shyam Prasad N @ 2024-12-19 9:33 UTC (permalink / raw) To: Steve French Cc: Bharath SM, linux-cifs, sfrench, pc, sprasad, tom, ronniesahlberg, Bharath SM On Tue, Dec 17, 2024 at 2:22 AM Steve French <smfrench@gmail.com> wrote: > > merged into cifs-2.6.git for-next pending review and more testing > > On Mon, Dec 16, 2024 at 12:36 PM Bharath SM <bharathsm.hsk@gmail.com> wrote: > > > > Previously, deferred file handles were reused only for read > > operations, this commit extends to reusing deferred handles > > for write operations. > > > > Signed-off-by: Bharath SM <bharathsm@microsoft.com> > > --- > > fs/smb/client/file.c | 6 +++++- > > 1 file changed, 5 insertions(+), 1 deletion(-) > > > > diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c > > index a58a3333ecc3..98deff1de74c 100644 > > --- a/fs/smb/client/file.c > > +++ b/fs/smb/client/file.c > > @@ -990,7 +990,11 @@ int cifs_open(struct inode *inode, struct file *file) > > } > > > > /* Get the cached handle as SMB2 close is deferred */ > > - rc = cifs_get_readable_path(tcon, full_path, &cfile); > > + if (OPEN_FMODE(file->f_flags) & FMODE_WRITE) { > > + rc = cifs_get_writable_path(tcon, full_path, FIND_WR_ANY, &cfile); Wondering if FIND_WR_ANY is okay for all use cases? Specifically, I'm checking where FIND_WR_FSUID_ONLY is relevant. @Steve French Is this for multiuser mounts? I don't think so, since multiuser mounts come with their own tcon, and we search writable files in our tcon's open list. > > + } else { > > + rc = cifs_get_readable_path(tcon, full_path, &cfile); > > + } > > if (rc == 0) { > > if (file->f_flags == cfile->f_flags) { > > file->private_data = cfile; > > -- > > 2.43.0 > > > > > > > -- > Thanks, > > Steve > Other than that one thing to look at, the changes look good to me. -- Regards, Shyam ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] smb: enable reuse of deferred file handles for write operations 2024-12-19 9:33 ` Shyam Prasad N @ 2024-12-19 22:47 ` Paul Aurich 2024-12-23 8:12 ` Bharath SM 0 siblings, 1 reply; 6+ messages in thread From: Paul Aurich @ 2024-12-19 22:47 UTC (permalink / raw) To: Shyam Prasad N Cc: Steve French, Bharath SM, linux-cifs, sfrench, pc, sprasad, tom, ronniesahlberg, Bharath SM On 2024-12-19 15:03:48 +0530, Shyam Prasad N wrote: >On Tue, Dec 17, 2024 at 2:22 AM Steve French <smfrench@gmail.com> wrote: >> >> merged into cifs-2.6.git for-next pending review and more testing >> >> On Mon, Dec 16, 2024 at 12:36 PM Bharath SM <bharathsm.hsk@gmail.com> wrote: >> > >> > Previously, deferred file handles were reused only for read >> > operations, this commit extends to reusing deferred handles >> > for write operations. >> > >> > Signed-off-by: Bharath SM <bharathsm@microsoft.com> >> > --- >> > fs/smb/client/file.c | 6 +++++- >> > 1 file changed, 5 insertions(+), 1 deletion(-) >> > >> > diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c >> > index a58a3333ecc3..98deff1de74c 100644 >> > --- a/fs/smb/client/file.c >> > +++ b/fs/smb/client/file.c >> > @@ -990,7 +990,11 @@ int cifs_open(struct inode *inode, struct file *file) >> > } >> > >> > /* Get the cached handle as SMB2 close is deferred */ >> > - rc = cifs_get_readable_path(tcon, full_path, &cfile); >> > + if (OPEN_FMODE(file->f_flags) & FMODE_WRITE) { >> > + rc = cifs_get_writable_path(tcon, full_path, FIND_WR_ANY, &cfile); > >Wondering if FIND_WR_ANY is okay for all use cases? >Specifically, I'm checking where FIND_WR_FSUID_ONLY is relevant. >@Steve French Is this for multiuser mounts? I don't think so, since >multiuser mounts come with their own tcon, and we search writable >files in our tcon's open list. I think this should be FIND_WR_FSUID_ONLY, yeah. (IMHO, that should be the default, and FIND_WR_ANY should be renamed something indicating it should only be used in specific situations and it's probably not what the caller wants.) I have a series I need to resurrect and polish that fixes a few problems along these lines, but it doesn't touch the 'writable file' path. >> > + } else { >> > + rc = cifs_get_readable_path(tcon, full_path, &cfile); >> > + } >> > if (rc == 0) { >> > if (file->f_flags == cfile->f_flags) { >> > file->private_data = cfile; >> > -- >> > 2.43.0 >> > >> > >> >> >> -- >> Thanks, >> >> Steve >> >Other than that one thing to look at, the changes look good to me. > >-- >Regards, >Shyam ~Paul ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] smb: enable reuse of deferred file handles for write operations 2024-12-19 22:47 ` Paul Aurich @ 2024-12-23 8:12 ` Bharath SM 2024-12-23 18:48 ` Steve French 0 siblings, 1 reply; 6+ messages in thread From: Bharath SM @ 2024-12-23 8:12 UTC (permalink / raw) To: Shyam Prasad N, Steve French, Bharath SM, linux-cifs, sfrench, pc, sprasad, tom, ronniesahlberg, Bharath SM [-- Attachment #1: Type: text/plain, Size: 2630 bytes --] Thanks Shyam and Paul Aurich for reviewing and sharing comments.! I have attached an updated patch. On Fri, Dec 20, 2024 at 4:17 AM Paul Aurich <paul@darkrain42.org> wrote: > > On 2024-12-19 15:03:48 +0530, Shyam Prasad N wrote: > >On Tue, Dec 17, 2024 at 2:22 AM Steve French <smfrench@gmail.com> wrote: > >> > >> merged into cifs-2.6.git for-next pending review and more testing > >> > >> On Mon, Dec 16, 2024 at 12:36 PM Bharath SM <bharathsm.hsk@gmail.com> wrote: > >> > > >> > Previously, deferred file handles were reused only for read > >> > operations, this commit extends to reusing deferred handles > >> > for write operations. > >> > > >> > Signed-off-by: Bharath SM <bharathsm@microsoft.com> > >> > --- > >> > fs/smb/client/file.c | 6 +++++- > >> > 1 file changed, 5 insertions(+), 1 deletion(-) > >> > > >> > diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c > >> > index a58a3333ecc3..98deff1de74c 100644 > >> > --- a/fs/smb/client/file.c > >> > +++ b/fs/smb/client/file.c > >> > @@ -990,7 +990,11 @@ int cifs_open(struct inode *inode, struct file *file) > >> > } > >> > > >> > /* Get the cached handle as SMB2 close is deferred */ > >> > - rc = cifs_get_readable_path(tcon, full_path, &cfile); > >> > + if (OPEN_FMODE(file->f_flags) & FMODE_WRITE) { > >> > + rc = cifs_get_writable_path(tcon, full_path, FIND_WR_ANY, &cfile); > > > >Wondering if FIND_WR_ANY is okay for all use cases? > >Specifically, I'm checking where FIND_WR_FSUID_ONLY is relevant. > >@Steve French Is this for multiuser mounts? I don't think so, since > >multiuser mounts come with their own tcon, and we search writable > >files in our tcon's open list. > > I think this should be FIND_WR_FSUID_ONLY, yeah. (IMHO, that should be the > default, and FIND_WR_ANY should be renamed something indicating it should only > be used in specific situations and it's probably not what the caller wants.) > > I have a series I need to resurrect and polish that fixes a few problems along > these lines, but it doesn't touch the 'writable file' path. > > >> > + } else { > >> > + rc = cifs_get_readable_path(tcon, full_path, &cfile); > >> > + } > >> > if (rc == 0) { > >> > if (file->f_flags == cfile->f_flags) { > >> > file->private_data = cfile; > >> > -- > >> > 2.43.0 > >> > > >> > > >> > >> > >> -- > >> Thanks, > >> > >> Steve > >> > >Other than that one thing to look at, the changes look good to me. > > > >-- > >Regards, > >Shyam > > ~Paul > [-- Attachment #2: 0001-smb-enable-reuse-of-deferred-file-handles-for-write-.patch --] [-- Type: application/octet-stream, Size: 1250 bytes --] From 8643af77857e3129924e788b4def9872798c1772 Mon Sep 17 00:00:00 2001 From: Bharath SM <bharathsm@microsoft.com> Date: Fri, 13 Dec 2024 22:50:21 +0530 Subject: [PATCH] smb: enable reuse of deferred file handles for write operations Previously, deferred file handles were reused only for read operations, this commit extends to reusing deferred handles for write operations. By reusing these handles we can reduce the need for open/close operations over the wire. Signed-off-by: Bharath SM <bharathsm@microsoft.com> --- fs/smb/client/file.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c index a58a3333ecc3..3b2d33291a7e 100644 --- a/fs/smb/client/file.c +++ b/fs/smb/client/file.c @@ -990,7 +990,11 @@ int cifs_open(struct inode *inode, struct file *file) } /* Get the cached handle as SMB2 close is deferred */ - rc = cifs_get_readable_path(tcon, full_path, &cfile); + if (OPEN_FMODE(file->f_flags) & FMODE_WRITE) { + rc = cifs_get_writable_path(tcon, full_path, FIND_WR_FSUID_ONLY, &cfile); + } else { + rc = cifs_get_readable_path(tcon, full_path, &cfile); + } if (rc == 0) { if (file->f_flags == cfile->f_flags) { file->private_data = cfile; -- 2.43.0 ^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH] smb: enable reuse of deferred file handles for write operations 2024-12-23 8:12 ` Bharath SM @ 2024-12-23 18:48 ` Steve French 0 siblings, 0 replies; 6+ messages in thread From: Steve French @ 2024-12-23 18:48 UTC (permalink / raw) To: Bharath SM Cc: Shyam Prasad N, linux-cifs, pc, sprasad, tom, ronniesahlberg, Bharath SM Merged into cifs-2.6.git for-next pending more testing On Mon, Dec 23, 2024 at 2:12 AM Bharath SM <bharathsm.hsk@gmail.com> wrote: > > Thanks Shyam and Paul Aurich for reviewing and sharing comments.! > I have attached an updated patch. > > On Fri, Dec 20, 2024 at 4:17 AM Paul Aurich <paul@darkrain42.org> wrote: > > > > On 2024-12-19 15:03:48 +0530, Shyam Prasad N wrote: > > >On Tue, Dec 17, 2024 at 2:22 AM Steve French <smfrench@gmail.com> wrote: > > >> > > >> merged into cifs-2.6.git for-next pending review and more testing > > >> > > >> On Mon, Dec 16, 2024 at 12:36 PM Bharath SM <bharathsm.hsk@gmail.com> wrote: > > >> > > > >> > Previously, deferred file handles were reused only for read > > >> > operations, this commit extends to reusing deferred handles > > >> > for write operations. > > >> > > > >> > Signed-off-by: Bharath SM <bharathsm@microsoft.com> > > >> > --- > > >> > fs/smb/client/file.c | 6 +++++- > > >> > 1 file changed, 5 insertions(+), 1 deletion(-) > > >> > > > >> > diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c > > >> > index a58a3333ecc3..98deff1de74c 100644 > > >> > --- a/fs/smb/client/file.c > > >> > +++ b/fs/smb/client/file.c > > >> > @@ -990,7 +990,11 @@ int cifs_open(struct inode *inode, struct file *file) > > >> > } > > >> > > > >> > /* Get the cached handle as SMB2 close is deferred */ > > >> > - rc = cifs_get_readable_path(tcon, full_path, &cfile); > > >> > + if (OPEN_FMODE(file->f_flags) & FMODE_WRITE) { > > >> > + rc = cifs_get_writable_path(tcon, full_path, FIND_WR_ANY, &cfile); > > > > > >Wondering if FIND_WR_ANY is okay for all use cases? > > >Specifically, I'm checking where FIND_WR_FSUID_ONLY is relevant. > > >@Steve French Is this for multiuser mounts? I don't think so, since > > >multiuser mounts come with their own tcon, and we search writable > > >files in our tcon's open list. > > > > I think this should be FIND_WR_FSUID_ONLY, yeah. (IMHO, that should be the > > default, and FIND_WR_ANY should be renamed something indicating it should only > > be used in specific situations and it's probably not what the caller wants.) > > > > I have a series I need to resurrect and polish that fixes a few problems along > > these lines, but it doesn't touch the 'writable file' path. > > > > >> > + } else { > > >> > + rc = cifs_get_readable_path(tcon, full_path, &cfile); > > >> > + } > > >> > if (rc == 0) { > > >> > if (file->f_flags == cfile->f_flags) { > > >> > file->private_data = cfile; > > >> > -- > > >> > 2.43.0 > > >> > > > >> > > > >> > > >> > > >> -- > > >> Thanks, > > >> > > >> Steve > > >> > > >Other than that one thing to look at, the changes look good to me. > > > > > >-- > > >Regards, > > >Shyam > > > > ~Paul > > -- Thanks, Steve ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2024-12-23 18:48 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-12-16 18:31 [PATCH] smb: enable reuse of deferred file handles for write operations Bharath SM 2024-12-16 20:43 ` Steve French 2024-12-19 9:33 ` Shyam Prasad N 2024-12-19 22:47 ` Paul Aurich 2024-12-23 8:12 ` Bharath SM 2024-12-23 18:48 ` Steve French
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox