| Message ID | 20250429151827.1677612-1-pc@manguebit.com |
|---|---|
| State | New |
| Headers | show |
| Series | smb: client: ensure aligned IO sizes | expand |
Do you have a repro example? Could this have negative performance impact to some servers? On Tue, Apr 29, 2025 at 10:18 AM Paulo Alcantara <pc@manguebit.com> wrote: > > Make all IO sizes multiple of PAGE_SIZE, either negotiated by the > server or passed through rsize, wsize and bsize mount options, to > prevent from breaking DIO reads and writes against servers that > enforce alignment as specified in MS-FSA 2.1.5.3 and 2.1.5.4. > > Cc: linux-cifs@vger.kernel.org > Cc: David Howells <dhowells@redhat.com> > Signed-off-by: Paulo Alcantara (Red Hat) <pc@manguebit.com> > --- > fs/smb/client/cifsglob.h | 2 ++ > fs/smb/client/connect.c | 23 +---------------------- > fs/smb/client/file.c | 6 ++---- > fs/smb/client/fs_context.c | 25 ++++++------------------- > fs/smb/client/fs_context.h | 32 ++++++++++++++++++++++++++++++++ > fs/smb/client/smb1ops.c | 8 ++++---- > fs/smb/client/smb2pdu.c | 8 ++------ > 7 files changed, 49 insertions(+), 55 deletions(-) > > diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h > index 3b32116b0b49..24c2cd9532a9 100644 > --- a/fs/smb/client/cifsglob.h > +++ b/fs/smb/client/cifsglob.h > @@ -1024,6 +1024,8 @@ compare_mid(__u16 mid, const struct smb_hdr *smb) > #define CIFS_DEFAULT_NON_POSIX_RSIZE (60 * 1024) > #define CIFS_DEFAULT_NON_POSIX_WSIZE (65536) > > +#define CIFS_IO_ALIGN(v) umax(round_down((v), PAGE_SIZE), PAGE_SIZE) > + > /* > * Macros to allow the TCP_Server_Info->net field and related code to drop out > * when CONFIG_NET_NS isn't set. > diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c > index df976ce6aed9..6bf04d9a5491 100644 > --- a/fs/smb/client/connect.c > +++ b/fs/smb/client/connect.c > @@ -3753,28 +3753,7 @@ int cifs_mount_get_tcon(struct cifs_mount_ctx *mnt_ctx) > } > } > > - /* > - * Clamp the rsize/wsize mount arguments if they are too big for the server > - * and set the rsize/wsize to the negotiated values if not passed in by > - * the user on mount > - */ > - if ((cifs_sb->ctx->wsize == 0) || > - (cifs_sb->ctx->wsize > server->ops->negotiate_wsize(tcon, ctx))) { > - cifs_sb->ctx->wsize = > - round_down(server->ops->negotiate_wsize(tcon, ctx), PAGE_SIZE); > - /* > - * in the very unlikely event that the server sent a max write size under PAGE_SIZE, > - * (which would get rounded down to 0) then reset wsize to absolute minimum eg 4096 > - */ > - if (cifs_sb->ctx->wsize == 0) { > - cifs_sb->ctx->wsize = PAGE_SIZE; > - cifs_dbg(VFS, "wsize too small, reset to minimum ie PAGE_SIZE, usually 4096\n"); > - } > - } > - if ((cifs_sb->ctx->rsize == 0) || > - (cifs_sb->ctx->rsize > server->ops->negotiate_rsize(tcon, ctx))) > - cifs_sb->ctx->rsize = server->ops->negotiate_rsize(tcon, ctx); > - > + cifs_negotiate_iosize(server, cifs_sb->ctx, tcon); > /* > * The cookie is initialized from volume info returned above. > * Inside cifs_fscache_get_super_cookie it checks > diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c > index 9e8f404b9e56..851b74f557c1 100644 > --- a/fs/smb/client/file.c > +++ b/fs/smb/client/file.c > @@ -160,10 +160,8 @@ static int cifs_prepare_read(struct netfs_io_subrequest *subreq) > server = cifs_pick_channel(tlink_tcon(req->cfile->tlink)->ses); > rdata->server = server; > > - if (cifs_sb->ctx->rsize == 0) > - cifs_sb->ctx->rsize = > - server->ops->negotiate_rsize(tlink_tcon(req->cfile->tlink), > - cifs_sb->ctx); > + cifs_negotiate_rsize(server, cifs_sb->ctx, > + tlink_tcon(req->cfile->tlink)); > > rc = server->ops->wait_mtu_credits(server, cifs_sb->ctx->rsize, > &size, &rdata->credits); > diff --git a/fs/smb/client/fs_context.c b/fs/smb/client/fs_context.c > index 2980941b9667..f8fef852a9fb 100644 > --- a/fs/smb/client/fs_context.c > +++ b/fs/smb/client/fs_context.c > @@ -1021,6 +1021,7 @@ static int smb3_reconfigure(struct fs_context *fc) > struct dentry *root = fc->root; > struct cifs_sb_info *cifs_sb = CIFS_SB(root->d_sb); > struct cifs_ses *ses = cifs_sb_master_tcon(cifs_sb)->ses; > + unsigned int rsize = ctx->rsize, wsize = ctx->wsize; > char *new_password = NULL, *new_password2 = NULL; > bool need_recon = false; > int rc; > @@ -1103,11 +1104,8 @@ static int smb3_reconfigure(struct fs_context *fc) > STEAL_STRING(cifs_sb, ctx, iocharset); > > /* if rsize or wsize not passed in on remount, use previous values */ > - if (ctx->rsize == 0) > - ctx->rsize = cifs_sb->ctx->rsize; > - if (ctx->wsize == 0) > - ctx->wsize = cifs_sb->ctx->wsize; > - > + ctx->rsize = !rsize ? cifs_sb->ctx->rsize : CIFS_IO_ALIGN(rsize); > + ctx->wsize = !wsize ? cifs_sb->ctx->wsize : CIFS_IO_ALIGN(wsize); > > smb3_cleanup_fs_context_contents(cifs_sb->ctx); > rc = smb3_fs_context_dup(cifs_sb->ctx, ctx); > @@ -1312,7 +1310,7 @@ static int smb3_fs_context_parse_param(struct fs_context *fc, > __func__); > goto cifs_parse_mount_err; > } > - ctx->bsize = result.uint_32; > + ctx->bsize = CIFS_IO_ALIGN(result.uint_32); > ctx->got_bsize = true; > break; > case Opt_rasize: > @@ -1336,24 +1334,13 @@ static int smb3_fs_context_parse_param(struct fs_context *fc, > ctx->rasize = result.uint_32; > break; > case Opt_rsize: > - ctx->rsize = result.uint_32; > + ctx->rsize = CIFS_IO_ALIGN(result.uint_32); > ctx->got_rsize = true; > ctx->vol_rsize = ctx->rsize; > break; > case Opt_wsize: > - ctx->wsize = result.uint_32; > + ctx->wsize = CIFS_IO_ALIGN(result.uint_32); > ctx->got_wsize = true; > - if (ctx->wsize % PAGE_SIZE != 0) { > - ctx->wsize = round_down(ctx->wsize, PAGE_SIZE); > - if (ctx->wsize == 0) { > - ctx->wsize = PAGE_SIZE; > - cifs_dbg(VFS, "wsize too small, reset to minimum %ld\n", PAGE_SIZE); > - } else { > - cifs_dbg(VFS, > - "wsize rounded down to %d to multiple of PAGE_SIZE %ld\n", > - ctx->wsize, PAGE_SIZE); > - } > - } > ctx->vol_wsize = ctx->wsize; > break; > case Opt_acregmax: > diff --git a/fs/smb/client/fs_context.h b/fs/smb/client/fs_context.h > index d1d29249bcdb..6ae51e27b4ce 100644 > --- a/fs/smb/client/fs_context.h > +++ b/fs/smb/client/fs_context.h > @@ -361,4 +361,36 @@ static inline void cifs_mount_unlock(void) > mutex_unlock(&cifs_mount_mutex); > } > > +static inline void cifs_negotiate_rsize(struct TCP_Server_Info *server, > + struct smb3_fs_context *ctx, > + struct cifs_tcon *tcon) > +{ > + unsigned int size; > + > + size = umax(server->ops->negotiate_rsize(tcon, ctx), PAGE_SIZE); > + if (ctx->rsize) > + size = umax(umin(ctx->rsize, size), PAGE_SIZE); > + ctx->rsize = round_down(size, PAGE_SIZE); > +} > + > +static inline void cifs_negotiate_wsize(struct TCP_Server_Info *server, > + struct smb3_fs_context *ctx, > + struct cifs_tcon *tcon) > +{ > + unsigned int size; > + > + size = umax(server->ops->negotiate_wsize(tcon, ctx), PAGE_SIZE); > + if (ctx->wsize) > + size = umax(umin(ctx->wsize, size), PAGE_SIZE); > + ctx->wsize = round_down(size, PAGE_SIZE); > +} > + > +static inline void cifs_negotiate_iosize(struct TCP_Server_Info *server, > + struct smb3_fs_context *ctx, > + struct cifs_tcon *tcon) > +{ > + cifs_negotiate_rsize(server, ctx, tcon); > + cifs_negotiate_wsize(server, ctx, tcon); > +} > + > #endif > diff --git a/fs/smb/client/smb1ops.c b/fs/smb/client/smb1ops.c > index 0adeec652dc1..b5c9915a97c8 100644 > --- a/fs/smb/client/smb1ops.c > +++ b/fs/smb/client/smb1ops.c > @@ -432,7 +432,7 @@ cifs_negotiate(const unsigned int xid, > } > > static unsigned int > -cifs_negotiate_wsize(struct cifs_tcon *tcon, struct smb3_fs_context *ctx) > +smb1_negotiate_wsize(struct cifs_tcon *tcon, struct smb3_fs_context *ctx) > { > __u64 unix_cap = le64_to_cpu(tcon->fsUnixInfo.Capability); > struct TCP_Server_Info *server = tcon->ses->server; > @@ -467,7 +467,7 @@ cifs_negotiate_wsize(struct cifs_tcon *tcon, struct smb3_fs_context *ctx) > } > > static unsigned int > -cifs_negotiate_rsize(struct cifs_tcon *tcon, struct smb3_fs_context *ctx) > +smb1_negotiate_rsize(struct cifs_tcon *tcon, struct smb3_fs_context *ctx) > { > __u64 unix_cap = le64_to_cpu(tcon->fsUnixInfo.Capability); > struct TCP_Server_Info *server = tcon->ses->server; > @@ -1161,8 +1161,8 @@ struct smb_version_operations smb1_operations = { > .check_trans2 = cifs_check_trans2, > .need_neg = cifs_need_neg, > .negotiate = cifs_negotiate, > - .negotiate_wsize = cifs_negotiate_wsize, > - .negotiate_rsize = cifs_negotiate_rsize, > + .negotiate_wsize = smb1_negotiate_wsize, > + .negotiate_rsize = smb1_negotiate_rsize, > .sess_setup = CIFS_SessSetup, > .logoff = CIFSSMBLogoff, > .tree_connect = CIFSTCon, > diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c > index c4d52bebd37d..4590beb549e9 100644 > --- a/fs/smb/client/smb2pdu.c > +++ b/fs/smb/client/smb2pdu.c > @@ -4092,12 +4092,8 @@ static void cifs_renegotiate_iosize(struct TCP_Server_Info *server, > return; > > spin_lock(&tcon->sb_list_lock); > - list_for_each_entry(cifs_sb, &tcon->cifs_sb_list, tcon_sb_link) { > - cifs_sb->ctx->rsize = > - server->ops->negotiate_rsize(tcon, cifs_sb->ctx); > - cifs_sb->ctx->wsize = > - server->ops->negotiate_wsize(tcon, cifs_sb->ctx); > - } > + list_for_each_entry(cifs_sb, &tcon->cifs_sb_list, tcon_sb_link) > + cifs_negotiate_iosize(server, cifs_sb->ctx, tcon); > spin_unlock(&tcon->sb_list_lock); > } > > -- > 2.49.0 >
Paulo Alcantara <pc@manguebit.com> wrote: > - if (ctx->rsize == 0) > - ctx->rsize = cifs_sb->ctx->rsize; > - if (ctx->wsize == 0) > - ctx->wsize = cifs_sb->ctx->wsize; > - > + ctx->rsize = !rsize ? cifs_sb->ctx->rsize : CIFS_IO_ALIGN(rsize); > + ctx->wsize = !wsize ? cifs_sb->ctx->wsize : CIFS_IO_ALIGN(wsize); I would flip the logic: ctx->rsize = rsize ? CIFS_IO_ALIGN(rsize) : cifs_sb->ctx->rsize; ctx->wsize = wsize ? CIFS_IO_ALIGN(wsize) : cifs_sb->ctx->wsize; that puts the default last. > case Opt_wsize: > - ctx->wsize = result.uint_32; > + ctx->wsize = CIFS_IO_ALIGN(result.uint_32); > ctx->got_wsize = true; > - if (ctx->wsize % PAGE_SIZE != 0) { > - ctx->wsize = round_down(ctx->wsize, PAGE_SIZE); > - if (ctx->wsize == 0) { > - ctx->wsize = PAGE_SIZE; > - cifs_dbg(VFS, "wsize too small, reset to minimum %ld\n", PAGE_SIZE); > - } else { > - cifs_dbg(VFS, > - "wsize rounded down to %d to multiple of PAGE_SIZE %ld\n", > - ctx->wsize, PAGE_SIZE); > - } > - } > ctx->vol_wsize = ctx->wsize; > break; Are you sure you want to get rid of the warning for the misconfiguration? David
Steve French <smfrench@gmail.com> writes: > Do you have a repro example? ---8<--- #define _GNU_SOURCE #include <stdio.h> #include <stdlib.h> #include <unistd.h> #include <fcntl.h> #include <errno.h> #define die(s) perror(s), exit(1) int main(int argc, char *argv[]) { void *buf; int ret; int fd; if (argc < 2) { fprintf(stderr, "%s: [file]\n", argv[0]); exit(1); } fd = open(argv[1], O_CREAT|O_WRONLY|O_TRUNC); if (fd == -1 && errno != EEXIST) die("open"); close(fd); buf = malloc(8*1024*1024); fd = open(argv[1], O_DIRECT|O_RDONLY); if (fd == -1) { free(buf); die("open"); } ret = read(fd, buf, 8*1024*1024); free(buf); if (ret < 0) die("read"); close(fd); return 0; } ---8<--- $ mount.cifs //win-srv/share /mnt/1 -o ...,rsize=65535 $ ./test /mnt/1/foo read: Invalid argument $ umount /mnt/1 $ mount.cifs //win-srv/share /mnt/1 -o ...,rsize=65536 $ ./test /mnt/1/foo $ > Could this have negative performance impact to some servers? I don't see how this would impact performance. The default values for rsize, wsize and bsize remain the same. We just want to make sure that those values, either passed through mount options or negotiated by the server are multiple of page size. Otherwise it will break IO to files opened with O_DIRECT which require offsets and lengths to be multiple of block size.
David Howells <dhowells@redhat.com> writes: > Paulo Alcantara <pc@manguebit.com> wrote: > >> - if (ctx->rsize == 0) >> - ctx->rsize = cifs_sb->ctx->rsize; >> - if (ctx->wsize == 0) >> - ctx->wsize = cifs_sb->ctx->wsize; >> - >> + ctx->rsize = !rsize ? cifs_sb->ctx->rsize : CIFS_IO_ALIGN(rsize); >> + ctx->wsize = !wsize ? cifs_sb->ctx->wsize : CIFS_IO_ALIGN(wsize); > > I would flip the logic: > > ctx->rsize = rsize ? CIFS_IO_ALIGN(rsize) : cifs_sb->ctx->rsize; > ctx->wsize = wsize ? CIFS_IO_ALIGN(wsize) : cifs_sb->ctx->wsize; > > that puts the default last. Sounds good. Will change. > >> case Opt_wsize: >> - ctx->wsize = result.uint_32; >> + ctx->wsize = CIFS_IO_ALIGN(result.uint_32); >> ctx->got_wsize = true; >> - if (ctx->wsize % PAGE_SIZE != 0) { >> - ctx->wsize = round_down(ctx->wsize, PAGE_SIZE); >> - if (ctx->wsize == 0) { >> - ctx->wsize = PAGE_SIZE; >> - cifs_dbg(VFS, "wsize too small, reset to minimum %ld\n", PAGE_SIZE); >> - } else { >> - cifs_dbg(VFS, >> - "wsize rounded down to %d to multiple of PAGE_SIZE %ld\n", >> - ctx->wsize, PAGE_SIZE); >> - } >> - } >> ctx->vol_wsize = ctx->wsize; >> break; > > Are you sure you want to get rid of the warning for the misconfiguration? I just found it annoying when mounting the share multiple times with an unaligned wsize value. I'll keep it but I'll have to warn for the other IO sizes as well. Printing them once with VFS|ONCE should be better.
diff --git a/fs/smb/client/cifsglob.h b/fs/smb/client/cifsglob.h index 3b32116b0b49..24c2cd9532a9 100644 --- a/fs/smb/client/cifsglob.h +++ b/fs/smb/client/cifsglob.h @@ -1024,6 +1024,8 @@ compare_mid(__u16 mid, const struct smb_hdr *smb) #define CIFS_DEFAULT_NON_POSIX_RSIZE (60 * 1024) #define CIFS_DEFAULT_NON_POSIX_WSIZE (65536) +#define CIFS_IO_ALIGN(v) umax(round_down((v), PAGE_SIZE), PAGE_SIZE) + /* * Macros to allow the TCP_Server_Info->net field and related code to drop out * when CONFIG_NET_NS isn't set. diff --git a/fs/smb/client/connect.c b/fs/smb/client/connect.c index df976ce6aed9..6bf04d9a5491 100644 --- a/fs/smb/client/connect.c +++ b/fs/smb/client/connect.c @@ -3753,28 +3753,7 @@ int cifs_mount_get_tcon(struct cifs_mount_ctx *mnt_ctx) } } - /* - * Clamp the rsize/wsize mount arguments if they are too big for the server - * and set the rsize/wsize to the negotiated values if not passed in by - * the user on mount - */ - if ((cifs_sb->ctx->wsize == 0) || - (cifs_sb->ctx->wsize > server->ops->negotiate_wsize(tcon, ctx))) { - cifs_sb->ctx->wsize = - round_down(server->ops->negotiate_wsize(tcon, ctx), PAGE_SIZE); - /* - * in the very unlikely event that the server sent a max write size under PAGE_SIZE, - * (which would get rounded down to 0) then reset wsize to absolute minimum eg 4096 - */ - if (cifs_sb->ctx->wsize == 0) { - cifs_sb->ctx->wsize = PAGE_SIZE; - cifs_dbg(VFS, "wsize too small, reset to minimum ie PAGE_SIZE, usually 4096\n"); - } - } - if ((cifs_sb->ctx->rsize == 0) || - (cifs_sb->ctx->rsize > server->ops->negotiate_rsize(tcon, ctx))) - cifs_sb->ctx->rsize = server->ops->negotiate_rsize(tcon, ctx); - + cifs_negotiate_iosize(server, cifs_sb->ctx, tcon); /* * The cookie is initialized from volume info returned above. * Inside cifs_fscache_get_super_cookie it checks diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c index 9e8f404b9e56..851b74f557c1 100644 --- a/fs/smb/client/file.c +++ b/fs/smb/client/file.c @@ -160,10 +160,8 @@ static int cifs_prepare_read(struct netfs_io_subrequest *subreq) server = cifs_pick_channel(tlink_tcon(req->cfile->tlink)->ses); rdata->server = server; - if (cifs_sb->ctx->rsize == 0) - cifs_sb->ctx->rsize = - server->ops->negotiate_rsize(tlink_tcon(req->cfile->tlink), - cifs_sb->ctx); + cifs_negotiate_rsize(server, cifs_sb->ctx, + tlink_tcon(req->cfile->tlink)); rc = server->ops->wait_mtu_credits(server, cifs_sb->ctx->rsize, &size, &rdata->credits); diff --git a/fs/smb/client/fs_context.c b/fs/smb/client/fs_context.c index 2980941b9667..f8fef852a9fb 100644 --- a/fs/smb/client/fs_context.c +++ b/fs/smb/client/fs_context.c @@ -1021,6 +1021,7 @@ static int smb3_reconfigure(struct fs_context *fc) struct dentry *root = fc->root; struct cifs_sb_info *cifs_sb = CIFS_SB(root->d_sb); struct cifs_ses *ses = cifs_sb_master_tcon(cifs_sb)->ses; + unsigned int rsize = ctx->rsize, wsize = ctx->wsize; char *new_password = NULL, *new_password2 = NULL; bool need_recon = false; int rc; @@ -1103,11 +1104,8 @@ static int smb3_reconfigure(struct fs_context *fc) STEAL_STRING(cifs_sb, ctx, iocharset); /* if rsize or wsize not passed in on remount, use previous values */ - if (ctx->rsize == 0) - ctx->rsize = cifs_sb->ctx->rsize; - if (ctx->wsize == 0) - ctx->wsize = cifs_sb->ctx->wsize; - + ctx->rsize = !rsize ? cifs_sb->ctx->rsize : CIFS_IO_ALIGN(rsize); + ctx->wsize = !wsize ? cifs_sb->ctx->wsize : CIFS_IO_ALIGN(wsize); smb3_cleanup_fs_context_contents(cifs_sb->ctx); rc = smb3_fs_context_dup(cifs_sb->ctx, ctx); @@ -1312,7 +1310,7 @@ static int smb3_fs_context_parse_param(struct fs_context *fc, __func__); goto cifs_parse_mount_err; } - ctx->bsize = result.uint_32; + ctx->bsize = CIFS_IO_ALIGN(result.uint_32); ctx->got_bsize = true; break; case Opt_rasize: @@ -1336,24 +1334,13 @@ static int smb3_fs_context_parse_param(struct fs_context *fc, ctx->rasize = result.uint_32; break; case Opt_rsize: - ctx->rsize = result.uint_32; + ctx->rsize = CIFS_IO_ALIGN(result.uint_32); ctx->got_rsize = true; ctx->vol_rsize = ctx->rsize; break; case Opt_wsize: - ctx->wsize = result.uint_32; + ctx->wsize = CIFS_IO_ALIGN(result.uint_32); ctx->got_wsize = true; - if (ctx->wsize % PAGE_SIZE != 0) { - ctx->wsize = round_down(ctx->wsize, PAGE_SIZE); - if (ctx->wsize == 0) { - ctx->wsize = PAGE_SIZE; - cifs_dbg(VFS, "wsize too small, reset to minimum %ld\n", PAGE_SIZE); - } else { - cifs_dbg(VFS, - "wsize rounded down to %d to multiple of PAGE_SIZE %ld\n", - ctx->wsize, PAGE_SIZE); - } - } ctx->vol_wsize = ctx->wsize; break; case Opt_acregmax: diff --git a/fs/smb/client/fs_context.h b/fs/smb/client/fs_context.h index d1d29249bcdb..6ae51e27b4ce 100644 --- a/fs/smb/client/fs_context.h +++ b/fs/smb/client/fs_context.h @@ -361,4 +361,36 @@ static inline void cifs_mount_unlock(void) mutex_unlock(&cifs_mount_mutex); } +static inline void cifs_negotiate_rsize(struct TCP_Server_Info *server, + struct smb3_fs_context *ctx, + struct cifs_tcon *tcon) +{ + unsigned int size; + + size = umax(server->ops->negotiate_rsize(tcon, ctx), PAGE_SIZE); + if (ctx->rsize) + size = umax(umin(ctx->rsize, size), PAGE_SIZE); + ctx->rsize = round_down(size, PAGE_SIZE); +} + +static inline void cifs_negotiate_wsize(struct TCP_Server_Info *server, + struct smb3_fs_context *ctx, + struct cifs_tcon *tcon) +{ + unsigned int size; + + size = umax(server->ops->negotiate_wsize(tcon, ctx), PAGE_SIZE); + if (ctx->wsize) + size = umax(umin(ctx->wsize, size), PAGE_SIZE); + ctx->wsize = round_down(size, PAGE_SIZE); +} + +static inline void cifs_negotiate_iosize(struct TCP_Server_Info *server, + struct smb3_fs_context *ctx, + struct cifs_tcon *tcon) +{ + cifs_negotiate_rsize(server, ctx, tcon); + cifs_negotiate_wsize(server, ctx, tcon); +} + #endif diff --git a/fs/smb/client/smb1ops.c b/fs/smb/client/smb1ops.c index 0adeec652dc1..b5c9915a97c8 100644 --- a/fs/smb/client/smb1ops.c +++ b/fs/smb/client/smb1ops.c @@ -432,7 +432,7 @@ cifs_negotiate(const unsigned int xid, } static unsigned int -cifs_negotiate_wsize(struct cifs_tcon *tcon, struct smb3_fs_context *ctx) +smb1_negotiate_wsize(struct cifs_tcon *tcon, struct smb3_fs_context *ctx) { __u64 unix_cap = le64_to_cpu(tcon->fsUnixInfo.Capability); struct TCP_Server_Info *server = tcon->ses->server; @@ -467,7 +467,7 @@ cifs_negotiate_wsize(struct cifs_tcon *tcon, struct smb3_fs_context *ctx) } static unsigned int -cifs_negotiate_rsize(struct cifs_tcon *tcon, struct smb3_fs_context *ctx) +smb1_negotiate_rsize(struct cifs_tcon *tcon, struct smb3_fs_context *ctx) { __u64 unix_cap = le64_to_cpu(tcon->fsUnixInfo.Capability); struct TCP_Server_Info *server = tcon->ses->server; @@ -1161,8 +1161,8 @@ struct smb_version_operations smb1_operations = { .check_trans2 = cifs_check_trans2, .need_neg = cifs_need_neg, .negotiate = cifs_negotiate, - .negotiate_wsize = cifs_negotiate_wsize, - .negotiate_rsize = cifs_negotiate_rsize, + .negotiate_wsize = smb1_negotiate_wsize, + .negotiate_rsize = smb1_negotiate_rsize, .sess_setup = CIFS_SessSetup, .logoff = CIFSSMBLogoff, .tree_connect = CIFSTCon, diff --git a/fs/smb/client/smb2pdu.c b/fs/smb/client/smb2pdu.c index c4d52bebd37d..4590beb549e9 100644 --- a/fs/smb/client/smb2pdu.c +++ b/fs/smb/client/smb2pdu.c @@ -4092,12 +4092,8 @@ static void cifs_renegotiate_iosize(struct TCP_Server_Info *server, return; spin_lock(&tcon->sb_list_lock); - list_for_each_entry(cifs_sb, &tcon->cifs_sb_list, tcon_sb_link) { - cifs_sb->ctx->rsize = - server->ops->negotiate_rsize(tcon, cifs_sb->ctx); - cifs_sb->ctx->wsize = - server->ops->negotiate_wsize(tcon, cifs_sb->ctx); - } + list_for_each_entry(cifs_sb, &tcon->cifs_sb_list, tcon_sb_link) + cifs_negotiate_iosize(server, cifs_sb->ctx, tcon); spin_unlock(&tcon->sb_list_lock); }
Make all IO sizes multiple of PAGE_SIZE, either negotiated by the server or passed through rsize, wsize and bsize mount options, to prevent from breaking DIO reads and writes against servers that enforce alignment as specified in MS-FSA 2.1.5.3 and 2.1.5.4. Cc: linux-cifs@vger.kernel.org Cc: David Howells <dhowells@redhat.com> Signed-off-by: Paulo Alcantara (Red Hat) <pc@manguebit.com> --- fs/smb/client/cifsglob.h | 2 ++ fs/smb/client/connect.c | 23 +---------------------- fs/smb/client/file.c | 6 ++---- fs/smb/client/fs_context.c | 25 ++++++------------------- fs/smb/client/fs_context.h | 32 ++++++++++++++++++++++++++++++++ fs/smb/client/smb1ops.c | 8 ++++---- fs/smb/client/smb2pdu.c | 8 ++------ 7 files changed, 49 insertions(+), 55 deletions(-)