* [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