| Message ID | 1461286320-24601-7-git-send-email-jeff.layton@primarydata.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
Hi Jeff, It looks like this patch removes the mirror->uid and mirror->gid parameters that your earlier patch (nfs: don't match flexfiles mirrors that have different creds) makes use of. Should that patch be updated, or does this series make that patch obsolete? Thanks, Anna On 04/21/2016 08:51 PM, Jeff Layton wrote: > We're just as likely to have allocation problems here as we would if we > delay looking up the credential like we currently do. Fix the code to > get a rpc_cred reference early, as soon as the mirror is set up. > > This allows us to eliminate the mirror early if there is a problem > getting an rpc credential. This also allows us to drop the uid/gid > from the layout_mirror struct as well. > > In the event that we find an existing mirror where this one would go, we > swap in the new creds unconditionally, and drop the reference to the old > one. > > Note that the old ff_layout_update_mirror_cred function wouldn't set > this pointer unless the DS version was 3, but we don't know what the DS > version is at this point. I'm a little unclear on why it did that as you > still need creds to talk to v4 servers as well. I have the code set > it regardless of the DS version here. > > Also note the change to using generic creds instead of calling > lookup_cred directly. With that change, we also need to populate the > group_info pointer in the acred as some functions expect that to never > be NULL. Instead of allocating one every time however, we can allocate > one when the module is loaded and share it since the group_info is > refcounted. > > Signed-off-by: Jeff Layton <jeff.layton@primarydata.com> > --- > fs/nfs/flexfilelayout/flexfilelayout.c | 41 ++++++++++++++++++++++----- > fs/nfs/flexfilelayout/flexfilelayout.h | 2 -- > fs/nfs/flexfilelayout/flexfilelayoutdev.c | 47 ++----------------------------- > 3 files changed, 36 insertions(+), 54 deletions(-) > > diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c b/fs/nfs/flexfilelayout/flexfilelayout.c > index 432d5b0009c7..19806caa8ff2 100644 > --- a/fs/nfs/flexfilelayout/flexfilelayout.c > +++ b/fs/nfs/flexfilelayout/flexfilelayout.c > @@ -26,6 +26,8 @@ > > #define FF_LAYOUT_POLL_RETRY_MAX (15*HZ) > > +static struct group_info *ff_zero_group; > + > static struct pnfs_layout_hdr * > ff_layout_alloc_layout_hdr(struct inode *inode, gfp_t gfp_flags) > { > @@ -407,8 +409,9 @@ ff_layout_alloc_lseg(struct pnfs_layout_hdr *lh, > struct nfs4_ff_layout_mirror *mirror; > struct nfs4_deviceid devid; > struct nfs4_deviceid_node *idnode; > - u32 ds_count; > - u32 fh_count; > + struct auth_cred acred = { .group_info = ff_zero_group }; > + struct rpc_cred *cred; > + u32 ds_count, fh_count, id; > int j; > > rc = -EIO; > @@ -484,24 +487,39 @@ ff_layout_alloc_lseg(struct pnfs_layout_hdr *lh, > fls->mirror_array[i]->fh_versions_cnt = fh_count; > > /* user */ > - rc = decode_name(&stream, &fls->mirror_array[i]->uid); > + rc = decode_name(&stream, &id); > if (rc) > goto out_err_free; > > + acred.uid = make_kuid(&init_user_ns, id); > + > /* group */ > - rc = decode_name(&stream, &fls->mirror_array[i]->gid); > + rc = decode_name(&stream, &id); > if (rc) > goto out_err_free; > > + acred.gid = make_kgid(&init_user_ns, id); > + > + /* find the cred for it */ > + cred = rpc_lookup_generic_cred(&acred, 0, gfp_flags); > + if (IS_ERR(cred)) { > + rc = PTR_ERR(cred); > + goto out_err_free; > + } > + > + rcu_assign_pointer(fls->mirror_array[i]->cred, cred); > + > mirror = ff_layout_add_mirror(lh, fls->mirror_array[i]); > if (mirror != fls->mirror_array[i]) { > + /* swap cred ptrs so free_mirror will clean up old */ > + fls->mirror_array[i]->cred = xchg(&mirror->cred, cred); > ff_layout_free_mirror(fls->mirror_array[i]); > fls->mirror_array[i] = mirror; > } > > - dprintk("%s: uid %d gid %d\n", __func__, > - fls->mirror_array[i]->uid, > - fls->mirror_array[i]->gid); > + dprintk("%s: uid %u gid %u\n", __func__, > + from_kuid(&init_user_ns, acred.uid), > + from_kgid(&init_user_ns, acred.gid)); > } > > p = xdr_inline_decode(&stream, 4); > @@ -2226,6 +2244,11 @@ static int __init nfs4flexfilelayout_init(void) > { > printk(KERN_INFO "%s: NFSv4 Flexfile Layout Driver Registering...\n", > __func__); > + if (!ff_zero_group) { > + ff_zero_group = groups_alloc(0); > + if (!ff_zero_group) > + return -ENOMEM; > + } > return pnfs_register_layoutdriver(&flexfilelayout_type); > } > > @@ -2234,6 +2257,10 @@ static void __exit nfs4flexfilelayout_exit(void) > printk(KERN_INFO "%s: NFSv4 Flexfile Layout Driver Unregistering...\n", > __func__); > pnfs_unregister_layoutdriver(&flexfilelayout_type); > + if (ff_zero_group) { > + put_group_info(ff_zero_group); > + ff_zero_group = NULL; > + } > } > > MODULE_ALIAS("nfs-layouttype4-4"); > diff --git a/fs/nfs/flexfilelayout/flexfilelayout.h b/fs/nfs/flexfilelayout/flexfilelayout.h > index dd353bb7dc0a..c29fc853ce74 100644 > --- a/fs/nfs/flexfilelayout/flexfilelayout.h > +++ b/fs/nfs/flexfilelayout/flexfilelayout.h > @@ -76,8 +76,6 @@ struct nfs4_ff_layout_mirror { > u32 fh_versions_cnt; > struct nfs_fh *fh_versions; > nfs4_stateid stateid; > - u32 uid; > - u32 gid; > struct rpc_cred *cred; > atomic_t ref; > spinlock_t lock; > diff --git a/fs/nfs/flexfilelayout/flexfilelayoutdev.c b/fs/nfs/flexfilelayout/flexfilelayoutdev.c > index baee22929174..6ddd8a5c5ae0 100644 > --- a/fs/nfs/flexfilelayout/flexfilelayoutdev.c > +++ b/fs/nfs/flexfilelayout/flexfilelayoutdev.c > @@ -302,42 +302,6 @@ int ff_layout_track_ds_error(struct nfs4_flexfile_layout *flo, > return 0; > } > > -/* currently we only support AUTH_NONE and AUTH_SYS */ > -static rpc_authflavor_t > -nfs4_ff_layout_choose_authflavor(struct nfs4_ff_layout_mirror *mirror) > -{ > - if (mirror->uid == (u32)-1) > - return RPC_AUTH_NULL; > - return RPC_AUTH_UNIX; > -} > - > -/* fetch cred for NFSv3 DS */ > -static int ff_layout_update_mirror_cred(struct nfs4_ff_layout_mirror *mirror, > - struct nfs4_pnfs_ds *ds) > -{ > - if (ds->ds_clp && !mirror->cred && > - mirror->mirror_ds->ds_versions[0].version == 3) { > - struct rpc_auth *auth = ds->ds_clp->cl_rpcclient->cl_auth; > - struct rpc_cred *cred; > - struct auth_cred acred = { > - .uid = make_kuid(&init_user_ns, mirror->uid), > - .gid = make_kgid(&init_user_ns, mirror->gid), > - }; > - > - /* AUTH_NULL ignores acred */ > - cred = auth->au_ops->lookup_cred(auth, &acred, 0); > - if (IS_ERR(cred)) { > - dprintk("%s: lookup_cred failed with %ld\n", > - __func__, PTR_ERR(cred)); > - return PTR_ERR(cred); > - } else { > - if (cmpxchg(&mirror->cred, NULL, cred)) > - put_rpccred(cred); > - } > - } > - return 0; > -} > - > static struct rpc_cred * > ff_layout_get_mirror_cred(struct nfs4_ff_layout_mirror *mirror, u32 iomode) > { > @@ -386,7 +350,6 @@ nfs4_ff_layout_prepare_ds(struct pnfs_layout_segment *lseg, u32 ds_idx, > struct inode *ino = lseg->pls_layout->plh_inode; > struct nfs_server *s = NFS_SERVER(ino); > unsigned int max_payload; > - rpc_authflavor_t flavor; > > if (!ff_layout_mirror_valid(lseg, mirror)) { > pr_err_ratelimited("NFS: %s: No data server for offset index %d\n", > @@ -402,9 +365,7 @@ nfs4_ff_layout_prepare_ds(struct pnfs_layout_segment *lseg, u32 ds_idx, > /* matching smp_wmb() in _nfs4_pnfs_v3/4_ds_connect */ > smp_rmb(); > if (ds->ds_clp) > - goto out_update_creds; > - > - flavor = nfs4_ff_layout_choose_authflavor(mirror); > + goto out; > > /* FIXME: For now we assume the server sent only one version of NFS > * to use for the DS. > @@ -413,7 +374,7 @@ nfs4_ff_layout_prepare_ds(struct pnfs_layout_segment *lseg, u32 ds_idx, > dataserver_retrans, > mirror->mirror_ds->ds_versions[0].version, > mirror->mirror_ds->ds_versions[0].minor_version, > - flavor); > + RPC_AUTH_UNIX); > > /* connect success, check rsize/wsize limit */ > if (ds->ds_clp) { > @@ -438,11 +399,7 @@ nfs4_ff_layout_prepare_ds(struct pnfs_layout_segment *lseg, u32 ds_idx, > } else > pnfs_error_mark_layout_for_return(ino, lseg); > ds = NULL; > - goto out; > } > -out_update_creds: > - if (ff_layout_update_mirror_cred(mirror, ds)) > - ds = NULL; > out: > return ds; > } > -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Oh! The original patch should be dropped. I should have sent a self-NAK on it before. I'll do that now... Thanks, Jeff On Thu, 2016-04-28 at 16:37 -0400, Anna Schumaker wrote: > Hi Jeff, > > It looks like this patch removes the mirror->uid and mirror->gid > parameters that your earlier patch (nfs: don't match flexfiles > mirrors that have different creds) makes use of. Should that patch > be updated, or does this series make that patch obsolete? > > Thanks, > Anna > > On 04/21/2016 08:51 PM, Jeff Layton wrote: > > We're just as likely to have allocation problems here as we would > > if we > > delay looking up the credential like we currently do. Fix the code > > to > > get a rpc_cred reference early, as soon as the mirror is set up. > > > > This allows us to eliminate the mirror early if there is a problem > > getting an rpc credential. This also allows us to drop the uid/gid > > from the layout_mirror struct as well. > > > > In the event that we find an existing mirror where this one would > > go, we > > swap in the new creds unconditionally, and drop the reference to > > the old > > one. > > > > Note that the old ff_layout_update_mirror_cred function wouldn't > > set > > this pointer unless the DS version was 3, but we don't know what > > the DS > > version is at this point. I'm a little unclear on why it did that > > as you > > still need creds to talk to v4 servers as well. I have the code set > > it regardless of the DS version here. > > > > Also note the change to using generic creds instead of calling > > lookup_cred directly. With that change, we also need to populate > > the > > group_info pointer in the acred as some functions expect that to > > never > > be NULL. Instead of allocating one every time however, we can > > allocate > > one when the module is loaded and share it since the group_info is > > refcounted. > > > > Signed-off-by: Jeff Layton <jeff.layton@primarydata.com> > > --- > > fs/nfs/flexfilelayout/flexfilelayout.c | 41 > > ++++++++++++++++++++++----- > > fs/nfs/flexfilelayout/flexfilelayout.h | 2 -- > > fs/nfs/flexfilelayout/flexfilelayoutdev.c | 47 ++----------------- > > ------------ > > 3 files changed, 36 insertions(+), 54 deletions(-) > > > > diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c > > b/fs/nfs/flexfilelayout/flexfilelayout.c > > index 432d5b0009c7..19806caa8ff2 100644 > > --- a/fs/nfs/flexfilelayout/flexfilelayout.c > > +++ b/fs/nfs/flexfilelayout/flexfilelayout.c > > @@ -26,6 +26,8 @@ > > > > #define FF_LAYOUT_POLL_RETRY_MAX (15*HZ) > > > > +static struct group_info *ff_zero_group; > > + > > static struct pnfs_layout_hdr * > > ff_layout_alloc_layout_hdr(struct inode *inode, gfp_t gfp_flags) > > { > > @@ -407,8 +409,9 @@ ff_layout_alloc_lseg(struct pnfs_layout_hdr > > *lh, > > struct nfs4_ff_layout_mirror *mirror; > > struct nfs4_deviceid devid; > > struct nfs4_deviceid_node *idnode; > > - u32 ds_count; > > - u32 fh_count; > > + struct auth_cred acred = { .group_info = > > ff_zero_group }; > > + struct rpc_cred *cred; > > + u32 ds_count, fh_count, id; > > int j; > > > > rc = -EIO; > > @@ -484,24 +487,39 @@ ff_layout_alloc_lseg(struct pnfs_layout_hdr > > *lh, > > fls->mirror_array[i]->fh_versions_cnt = fh_count; > > > > /* user */ > > - rc = decode_name(&stream, &fls->mirror_array[i]- > > >uid); > > + rc = decode_name(&stream, &id); > > if (rc) > > goto out_err_free; > > > > + acred.uid = make_kuid(&init_user_ns, id); > > + > > /* group */ > > - rc = decode_name(&stream, &fls->mirror_array[i]- > > >gid); > > + rc = decode_name(&stream, &id); > > if (rc) > > goto out_err_free; > > > > + acred.gid = make_kgid(&init_user_ns, id); > > + > > + /* find the cred for it */ > > + cred = rpc_lookup_generic_cred(&acred, 0, > > gfp_flags); > > + if (IS_ERR(cred)) { > > + rc = PTR_ERR(cred); > > + goto out_err_free; > > + } > > + > > + rcu_assign_pointer(fls->mirror_array[i]->cred, > > cred); > > + > > mirror = ff_layout_add_mirror(lh, fls- > > >mirror_array[i]); > > if (mirror != fls->mirror_array[i]) { > > + /* swap cred ptrs so free_mirror will > > clean up old */ > > + fls->mirror_array[i]->cred = xchg(&mirror- > > >cred, cred); > > ff_layout_free_mirror(fls- > > >mirror_array[i]); > > fls->mirror_array[i] = mirror; > > } > > > > - dprintk("%s: uid %d gid %d\n", __func__, > > - fls->mirror_array[i]->uid, > > - fls->mirror_array[i]->gid); > > + dprintk("%s: uid %u gid %u\n", __func__, > > + from_kuid(&init_user_ns, acred.uid), > > + from_kgid(&init_user_ns, acred.gid)); > > } > > > > p = xdr_inline_decode(&stream, 4); > > @@ -2226,6 +2244,11 @@ static int __init > > nfs4flexfilelayout_init(void) > > { > > printk(KERN_INFO "%s: NFSv4 Flexfile Layout Driver > > Registering...\n", > > __func__); > > + if (!ff_zero_group) { > > + ff_zero_group = groups_alloc(0); > > + if (!ff_zero_group) > > + return -ENOMEM; > > + } > > return pnfs_register_layoutdriver(&flexfilelayout_type); > > } > > > > @@ -2234,6 +2257,10 @@ static void __exit > > nfs4flexfilelayout_exit(void) > > printk(KERN_INFO "%s: NFSv4 Flexfile Layout Driver > > Unregistering...\n", > > __func__); > > pnfs_unregister_layoutdriver(&flexfilelayout_type); > > + if (ff_zero_group) { > > + put_group_info(ff_zero_group); > > + ff_zero_group = NULL; > > + } > > } > > > > MODULE_ALIAS("nfs-layouttype4-4"); > > diff --git a/fs/nfs/flexfilelayout/flexfilelayout.h > > b/fs/nfs/flexfilelayout/flexfilelayout.h > > index dd353bb7dc0a..c29fc853ce74 100644 > > --- a/fs/nfs/flexfilelayout/flexfilelayout.h > > +++ b/fs/nfs/flexfilelayout/flexfilelayout.h > > @@ -76,8 +76,6 @@ struct nfs4_ff_layout_mirror { > > u32 fh_versions_cnt; > > struct nfs_fh *fh_versions; > > nfs4_stateid stateid; > > - u32 uid; > > - u32 gid; > > struct rpc_cred *cred; > > atomic_t ref; > > spinlock_t lock; > > diff --git a/fs/nfs/flexfilelayout/flexfilelayoutdev.c > > b/fs/nfs/flexfilelayout/flexfilelayoutdev.c > > index baee22929174..6ddd8a5c5ae0 100644 > > --- a/fs/nfs/flexfilelayout/flexfilelayoutdev.c > > +++ b/fs/nfs/flexfilelayout/flexfilelayoutdev.c > > @@ -302,42 +302,6 @@ int ff_layout_track_ds_error(struct > > nfs4_flexfile_layout *flo, > > return 0; > > } > > > > -/* currently we only support AUTH_NONE and AUTH_SYS */ > > -static rpc_authflavor_t > > -nfs4_ff_layout_choose_authflavor(struct nfs4_ff_layout_mirror > > *mirror) > > -{ > > - if (mirror->uid == (u32)-1) > > - return RPC_AUTH_NULL; > > - return RPC_AUTH_UNIX; > > -} > > - > > -/* fetch cred for NFSv3 DS */ > > -static int ff_layout_update_mirror_cred(struct > > nfs4_ff_layout_mirror *mirror, > > - struct nfs4_pnfs_ds *ds) > > -{ > > - if (ds->ds_clp && !mirror->cred && > > - mirror->mirror_ds->ds_versions[0].version == 3) { > > - struct rpc_auth *auth = ds->ds_clp->cl_rpcclient- > > >cl_auth; > > - struct rpc_cred *cred; > > - struct auth_cred acred = { > > - .uid = make_kuid(&init_user_ns, mirror- > > >uid), > > - .gid = make_kgid(&init_user_ns, mirror- > > >gid), > > - }; > > - > > - /* AUTH_NULL ignores acred */ > > - cred = auth->au_ops->lookup_cred(auth, &acred, 0); > > - if (IS_ERR(cred)) { > > - dprintk("%s: lookup_cred failed with > > %ld\n", > > - __func__, PTR_ERR(cred)); > > - return PTR_ERR(cred); > > - } else { > > - if (cmpxchg(&mirror->cred, NULL, cred)) > > - put_rpccred(cred); > > - } > > - } > > - return 0; > > -} > > - > > static struct rpc_cred * > > ff_layout_get_mirror_cred(struct nfs4_ff_layout_mirror *mirror, > > u32 iomode) > > { > > @@ -386,7 +350,6 @@ nfs4_ff_layout_prepare_ds(struct > > pnfs_layout_segment *lseg, u32 ds_idx, > > struct inode *ino = lseg->pls_layout->plh_inode; > > struct nfs_server *s = NFS_SERVER(ino); > > unsigned int max_payload; > > - rpc_authflavor_t flavor; > > > > if (!ff_layout_mirror_valid(lseg, mirror)) { > > pr_err_ratelimited("NFS: %s: No data server for > > offset index %d\n", > > @@ -402,9 +365,7 @@ nfs4_ff_layout_prepare_ds(struct > > pnfs_layout_segment *lseg, u32 ds_idx, > > /* matching smp_wmb() in _nfs4_pnfs_v3/4_ds_connect */ > > smp_rmb(); > > if (ds->ds_clp) > > - goto out_update_creds; > > - > > - flavor = nfs4_ff_layout_choose_authflavor(mirror); > > + goto out; > > > > /* FIXME: For now we assume the server sent only one > > version of NFS > > * to use for the DS. > > @@ -413,7 +374,7 @@ nfs4_ff_layout_prepare_ds(struct > > pnfs_layout_segment *lseg, u32 ds_idx, > > dataserver_retrans, > > mirror->mirror_ds- > > >ds_versions[0].version, > > mirror->mirror_ds- > > >ds_versions[0].minor_version, > > - flavor); > > + RPC_AUTH_UNIX); > > > > /* connect success, check rsize/wsize limit */ > > if (ds->ds_clp) { > > @@ -438,11 +399,7 @@ nfs4_ff_layout_prepare_ds(struct > > pnfs_layout_segment *lseg, u32 ds_idx, > > } else > > pnfs_error_mark_layout_for_return(ino, > > lseg); > > ds = NULL; > > - goto out; > > } > > -out_update_creds: > > - if (ff_layout_update_mirror_cred(mirror, ds)) > > - ds = NULL; > > out: > > return ds; > > } > > > > -- > To unsubscribe from this list: send the line "unsubscribe linux-nfs" > in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 04/29/2016 06:49 AM, Jeff Layton wrote: > Oh! The original patch should be dropped. I should have sent a self-NAK > on it before. I'll do that now... Great, thanks! Anna > > Thanks, > Jeff > > On Thu, 2016-04-28 at 16:37 -0400, Anna Schumaker wrote: >> Hi Jeff, >> >> It looks like this patch removes the mirror->uid and mirror->gid >> parameters that your earlier patch (nfs: don't match flexfiles >> mirrors that have different creds) makes use of. Should that patch >> be updated, or does this series make that patch obsolete? >> >> Thanks, >> Anna >> >> On 04/21/2016 08:51 PM, Jeff Layton wrote: >>> We're just as likely to have allocation problems here as we would >>> if we >>> delay looking up the credential like we currently do. Fix the code >>> to >>> get a rpc_cred reference early, as soon as the mirror is set up. >>> >>> This allows us to eliminate the mirror early if there is a problem >>> getting an rpc credential. This also allows us to drop the uid/gid >>> from the layout_mirror struct as well. >>> >>> In the event that we find an existing mirror where this one would >>> go, we >>> swap in the new creds unconditionally, and drop the reference to >>> the old >>> one. >>> >>> Note that the old ff_layout_update_mirror_cred function wouldn't >>> set >>> this pointer unless the DS version was 3, but we don't know what >>> the DS >>> version is at this point. I'm a little unclear on why it did that >>> as you >>> still need creds to talk to v4 servers as well. I have the code set >>> it regardless of the DS version here. >>> >>> Also note the change to using generic creds instead of calling >>> lookup_cred directly. With that change, we also need to populate >>> the >>> group_info pointer in the acred as some functions expect that to >>> never >>> be NULL. Instead of allocating one every time however, we can >>> allocate >>> one when the module is loaded and share it since the group_info is >>> refcounted. >>> >>> Signed-off-by: Jeff Layton <jeff.layton@primarydata.com> >>> --- >>> fs/nfs/flexfilelayout/flexfilelayout.c | 41 >>> ++++++++++++++++++++++----- >>> fs/nfs/flexfilelayout/flexfilelayout.h | 2 -- >>> fs/nfs/flexfilelayout/flexfilelayoutdev.c | 47 ++----------------- >>> ------------ >>> 3 files changed, 36 insertions(+), 54 deletions(-) >>> >>> diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c >>> b/fs/nfs/flexfilelayout/flexfilelayout.c >>> index 432d5b0009c7..19806caa8ff2 100644 >>> --- a/fs/nfs/flexfilelayout/flexfilelayout.c >>> +++ b/fs/nfs/flexfilelayout/flexfilelayout.c >>> @@ -26,6 +26,8 @@ >>> >>> #define FF_LAYOUT_POLL_RETRY_MAX (15*HZ) >>> >>> +static struct group_info *ff_zero_group; >>> + >>> static struct pnfs_layout_hdr * >>> ff_layout_alloc_layout_hdr(struct inode *inode, gfp_t gfp_flags) >>> { >>> @@ -407,8 +409,9 @@ ff_layout_alloc_lseg(struct pnfs_layout_hdr >>> *lh, >>> struct nfs4_ff_layout_mirror *mirror; >>> struct nfs4_deviceid devid; >>> struct nfs4_deviceid_node *idnode; >>> - u32 ds_count; >>> - u32 fh_count; >>> + struct auth_cred acred = { .group_info = >>> ff_zero_group }; >>> + struct rpc_cred *cred; >>> + u32 ds_count, fh_count, id; >>> int j; >>> >>> rc = -EIO; >>> @@ -484,24 +487,39 @@ ff_layout_alloc_lseg(struct pnfs_layout_hdr >>> *lh, >>> fls->mirror_array[i]->fh_versions_cnt = fh_count; >>> >>> /* user */ >>> - rc = decode_name(&stream, &fls->mirror_array[i]- >>>> uid); >>> + rc = decode_name(&stream, &id); >>> if (rc) >>> goto out_err_free; >>> >>> + acred.uid = make_kuid(&init_user_ns, id); >>> + >>> /* group */ >>> - rc = decode_name(&stream, &fls->mirror_array[i]- >>>> gid); >>> + rc = decode_name(&stream, &id); >>> if (rc) >>> goto out_err_free; >>> >>> + acred.gid = make_kgid(&init_user_ns, id); >>> + >>> + /* find the cred for it */ >>> + cred = rpc_lookup_generic_cred(&acred, 0, >>> gfp_flags); >>> + if (IS_ERR(cred)) { >>> + rc = PTR_ERR(cred); >>> + goto out_err_free; >>> + } >>> + >>> + rcu_assign_pointer(fls->mirror_array[i]->cred, >>> cred); >>> + >>> mirror = ff_layout_add_mirror(lh, fls- >>>> mirror_array[i]); >>> if (mirror != fls->mirror_array[i]) { >>> + /* swap cred ptrs so free_mirror will >>> clean up old */ >>> + fls->mirror_array[i]->cred = xchg(&mirror- >>>> cred, cred); >>> ff_layout_free_mirror(fls- >>>> mirror_array[i]); >>> fls->mirror_array[i] = mirror; >>> } >>> >>> - dprintk("%s: uid %d gid %d\n", __func__, >>> - fls->mirror_array[i]->uid, >>> - fls->mirror_array[i]->gid); >>> + dprintk("%s: uid %u gid %u\n", __func__, >>> + from_kuid(&init_user_ns, acred.uid), >>> + from_kgid(&init_user_ns, acred.gid)); >>> } >>> >>> p = xdr_inline_decode(&stream, 4); >>> @@ -2226,6 +2244,11 @@ static int __init >>> nfs4flexfilelayout_init(void) >>> { >>> printk(KERN_INFO "%s: NFSv4 Flexfile Layout Driver >>> Registering...\n", >>> __func__); >>> + if (!ff_zero_group) { >>> + ff_zero_group = groups_alloc(0); >>> + if (!ff_zero_group) >>> + return -ENOMEM; >>> + } >>> return pnfs_register_layoutdriver(&flexfilelayout_type); >>> } >>> >>> @@ -2234,6 +2257,10 @@ static void __exit >>> nfs4flexfilelayout_exit(void) >>> printk(KERN_INFO "%s: NFSv4 Flexfile Layout Driver >>> Unregistering...\n", >>> __func__); >>> pnfs_unregister_layoutdriver(&flexfilelayout_type); >>> + if (ff_zero_group) { >>> + put_group_info(ff_zero_group); >>> + ff_zero_group = NULL; >>> + } >>> } >>> >>> MODULE_ALIAS("nfs-layouttype4-4"); >>> diff --git a/fs/nfs/flexfilelayout/flexfilelayout.h >>> b/fs/nfs/flexfilelayout/flexfilelayout.h >>> index dd353bb7dc0a..c29fc853ce74 100644 >>> --- a/fs/nfs/flexfilelayout/flexfilelayout.h >>> +++ b/fs/nfs/flexfilelayout/flexfilelayout.h >>> @@ -76,8 +76,6 @@ struct nfs4_ff_layout_mirror { >>> u32 fh_versions_cnt; >>> struct nfs_fh *fh_versions; >>> nfs4_stateid stateid; >>> - u32 uid; >>> - u32 gid; >>> struct rpc_cred *cred; >>> atomic_t ref; >>> spinlock_t lock; >>> diff --git a/fs/nfs/flexfilelayout/flexfilelayoutdev.c >>> b/fs/nfs/flexfilelayout/flexfilelayoutdev.c >>> index baee22929174..6ddd8a5c5ae0 100644 >>> --- a/fs/nfs/flexfilelayout/flexfilelayoutdev.c >>> +++ b/fs/nfs/flexfilelayout/flexfilelayoutdev.c >>> @@ -302,42 +302,6 @@ int ff_layout_track_ds_error(struct >>> nfs4_flexfile_layout *flo, >>> return 0; >>> } >>> >>> -/* currently we only support AUTH_NONE and AUTH_SYS */ >>> -static rpc_authflavor_t >>> -nfs4_ff_layout_choose_authflavor(struct nfs4_ff_layout_mirror >>> *mirror) >>> -{ >>> - if (mirror->uid == (u32)-1) >>> - return RPC_AUTH_NULL; >>> - return RPC_AUTH_UNIX; >>> -} >>> - >>> -/* fetch cred for NFSv3 DS */ >>> -static int ff_layout_update_mirror_cred(struct >>> nfs4_ff_layout_mirror *mirror, >>> - struct nfs4_pnfs_ds *ds) >>> -{ >>> - if (ds->ds_clp && !mirror->cred && >>> - mirror->mirror_ds->ds_versions[0].version == 3) { >>> - struct rpc_auth *auth = ds->ds_clp->cl_rpcclient- >>>> cl_auth; >>> - struct rpc_cred *cred; >>> - struct auth_cred acred = { >>> - .uid = make_kuid(&init_user_ns, mirror- >>>> uid), >>> - .gid = make_kgid(&init_user_ns, mirror- >>>> gid), >>> - }; >>> - >>> - /* AUTH_NULL ignores acred */ >>> - cred = auth->au_ops->lookup_cred(auth, &acred, 0); >>> - if (IS_ERR(cred)) { >>> - dprintk("%s: lookup_cred failed with >>> %ld\n", >>> - __func__, PTR_ERR(cred)); >>> - return PTR_ERR(cred); >>> - } else { >>> - if (cmpxchg(&mirror->cred, NULL, cred)) >>> - put_rpccred(cred); >>> - } >>> - } >>> - return 0; >>> -} >>> - >>> static struct rpc_cred * >>> ff_layout_get_mirror_cred(struct nfs4_ff_layout_mirror *mirror, >>> u32 iomode) >>> { >>> @@ -386,7 +350,6 @@ nfs4_ff_layout_prepare_ds(struct >>> pnfs_layout_segment *lseg, u32 ds_idx, >>> struct inode *ino = lseg->pls_layout->plh_inode; >>> struct nfs_server *s = NFS_SERVER(ino); >>> unsigned int max_payload; >>> - rpc_authflavor_t flavor; >>> >>> if (!ff_layout_mirror_valid(lseg, mirror)) { >>> pr_err_ratelimited("NFS: %s: No data server for >>> offset index %d\n", >>> @@ -402,9 +365,7 @@ nfs4_ff_layout_prepare_ds(struct >>> pnfs_layout_segment *lseg, u32 ds_idx, >>> /* matching smp_wmb() in _nfs4_pnfs_v3/4_ds_connect */ >>> smp_rmb(); >>> if (ds->ds_clp) >>> - goto out_update_creds; >>> - >>> - flavor = nfs4_ff_layout_choose_authflavor(mirror); >>> + goto out; >>> >>> /* FIXME: For now we assume the server sent only one >>> version of NFS >>> * to use for the DS. >>> @@ -413,7 +374,7 @@ nfs4_ff_layout_prepare_ds(struct >>> pnfs_layout_segment *lseg, u32 ds_idx, >>> dataserver_retrans, >>> mirror->mirror_ds- >>>> ds_versions[0].version, >>> mirror->mirror_ds- >>>> ds_versions[0].minor_version, >>> - flavor); >>> + RPC_AUTH_UNIX); >>> >>> /* connect success, check rsize/wsize limit */ >>> if (ds->ds_clp) { >>> @@ -438,11 +399,7 @@ nfs4_ff_layout_prepare_ds(struct >>> pnfs_layout_segment *lseg, u32 ds_idx, >>> } else >>> pnfs_error_mark_layout_for_return(ino, >>> lseg); >>> ds = NULL; >>> - goto out; >>> } >>> -out_update_creds: >>> - if (ff_layout_update_mirror_cred(mirror, ds)) >>> - ds = NULL; >>> out: >>> return ds; >>> } >>> >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-nfs" >> in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html > -- > To unsubscribe from this list: send the line "unsubscribe linux-nfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c b/fs/nfs/flexfilelayout/flexfilelayout.c index 432d5b0009c7..19806caa8ff2 100644 --- a/fs/nfs/flexfilelayout/flexfilelayout.c +++ b/fs/nfs/flexfilelayout/flexfilelayout.c @@ -26,6 +26,8 @@ #define FF_LAYOUT_POLL_RETRY_MAX (15*HZ) +static struct group_info *ff_zero_group; + static struct pnfs_layout_hdr * ff_layout_alloc_layout_hdr(struct inode *inode, gfp_t gfp_flags) { @@ -407,8 +409,9 @@ ff_layout_alloc_lseg(struct pnfs_layout_hdr *lh, struct nfs4_ff_layout_mirror *mirror; struct nfs4_deviceid devid; struct nfs4_deviceid_node *idnode; - u32 ds_count; - u32 fh_count; + struct auth_cred acred = { .group_info = ff_zero_group }; + struct rpc_cred *cred; + u32 ds_count, fh_count, id; int j; rc = -EIO; @@ -484,24 +487,39 @@ ff_layout_alloc_lseg(struct pnfs_layout_hdr *lh, fls->mirror_array[i]->fh_versions_cnt = fh_count; /* user */ - rc = decode_name(&stream, &fls->mirror_array[i]->uid); + rc = decode_name(&stream, &id); if (rc) goto out_err_free; + acred.uid = make_kuid(&init_user_ns, id); + /* group */ - rc = decode_name(&stream, &fls->mirror_array[i]->gid); + rc = decode_name(&stream, &id); if (rc) goto out_err_free; + acred.gid = make_kgid(&init_user_ns, id); + + /* find the cred for it */ + cred = rpc_lookup_generic_cred(&acred, 0, gfp_flags); + if (IS_ERR(cred)) { + rc = PTR_ERR(cred); + goto out_err_free; + } + + rcu_assign_pointer(fls->mirror_array[i]->cred, cred); + mirror = ff_layout_add_mirror(lh, fls->mirror_array[i]); if (mirror != fls->mirror_array[i]) { + /* swap cred ptrs so free_mirror will clean up old */ + fls->mirror_array[i]->cred = xchg(&mirror->cred, cred); ff_layout_free_mirror(fls->mirror_array[i]); fls->mirror_array[i] = mirror; } - dprintk("%s: uid %d gid %d\n", __func__, - fls->mirror_array[i]->uid, - fls->mirror_array[i]->gid); + dprintk("%s: uid %u gid %u\n", __func__, + from_kuid(&init_user_ns, acred.uid), + from_kgid(&init_user_ns, acred.gid)); } p = xdr_inline_decode(&stream, 4); @@ -2226,6 +2244,11 @@ static int __init nfs4flexfilelayout_init(void) { printk(KERN_INFO "%s: NFSv4 Flexfile Layout Driver Registering...\n", __func__); + if (!ff_zero_group) { + ff_zero_group = groups_alloc(0); + if (!ff_zero_group) + return -ENOMEM; + } return pnfs_register_layoutdriver(&flexfilelayout_type); } @@ -2234,6 +2257,10 @@ static void __exit nfs4flexfilelayout_exit(void) printk(KERN_INFO "%s: NFSv4 Flexfile Layout Driver Unregistering...\n", __func__); pnfs_unregister_layoutdriver(&flexfilelayout_type); + if (ff_zero_group) { + put_group_info(ff_zero_group); + ff_zero_group = NULL; + } } MODULE_ALIAS("nfs-layouttype4-4"); diff --git a/fs/nfs/flexfilelayout/flexfilelayout.h b/fs/nfs/flexfilelayout/flexfilelayout.h index dd353bb7dc0a..c29fc853ce74 100644 --- a/fs/nfs/flexfilelayout/flexfilelayout.h +++ b/fs/nfs/flexfilelayout/flexfilelayout.h @@ -76,8 +76,6 @@ struct nfs4_ff_layout_mirror { u32 fh_versions_cnt; struct nfs_fh *fh_versions; nfs4_stateid stateid; - u32 uid; - u32 gid; struct rpc_cred *cred; atomic_t ref; spinlock_t lock; diff --git a/fs/nfs/flexfilelayout/flexfilelayoutdev.c b/fs/nfs/flexfilelayout/flexfilelayoutdev.c index baee22929174..6ddd8a5c5ae0 100644 --- a/fs/nfs/flexfilelayout/flexfilelayoutdev.c +++ b/fs/nfs/flexfilelayout/flexfilelayoutdev.c @@ -302,42 +302,6 @@ int ff_layout_track_ds_error(struct nfs4_flexfile_layout *flo, return 0; } -/* currently we only support AUTH_NONE and AUTH_SYS */ -static rpc_authflavor_t -nfs4_ff_layout_choose_authflavor(struct nfs4_ff_layout_mirror *mirror) -{ - if (mirror->uid == (u32)-1) - return RPC_AUTH_NULL; - return RPC_AUTH_UNIX; -} - -/* fetch cred for NFSv3 DS */ -static int ff_layout_update_mirror_cred(struct nfs4_ff_layout_mirror *mirror, - struct nfs4_pnfs_ds *ds) -{ - if (ds->ds_clp && !mirror->cred && - mirror->mirror_ds->ds_versions[0].version == 3) { - struct rpc_auth *auth = ds->ds_clp->cl_rpcclient->cl_auth; - struct rpc_cred *cred; - struct auth_cred acred = { - .uid = make_kuid(&init_user_ns, mirror->uid), - .gid = make_kgid(&init_user_ns, mirror->gid), - }; - - /* AUTH_NULL ignores acred */ - cred = auth->au_ops->lookup_cred(auth, &acred, 0); - if (IS_ERR(cred)) { - dprintk("%s: lookup_cred failed with %ld\n", - __func__, PTR_ERR(cred)); - return PTR_ERR(cred); - } else { - if (cmpxchg(&mirror->cred, NULL, cred)) - put_rpccred(cred); - } - } - return 0; -} - static struct rpc_cred * ff_layout_get_mirror_cred(struct nfs4_ff_layout_mirror *mirror, u32 iomode) { @@ -386,7 +350,6 @@ nfs4_ff_layout_prepare_ds(struct pnfs_layout_segment *lseg, u32 ds_idx, struct inode *ino = lseg->pls_layout->plh_inode; struct nfs_server *s = NFS_SERVER(ino); unsigned int max_payload; - rpc_authflavor_t flavor; if (!ff_layout_mirror_valid(lseg, mirror)) { pr_err_ratelimited("NFS: %s: No data server for offset index %d\n", @@ -402,9 +365,7 @@ nfs4_ff_layout_prepare_ds(struct pnfs_layout_segment *lseg, u32 ds_idx, /* matching smp_wmb() in _nfs4_pnfs_v3/4_ds_connect */ smp_rmb(); if (ds->ds_clp) - goto out_update_creds; - - flavor = nfs4_ff_layout_choose_authflavor(mirror); + goto out; /* FIXME: For now we assume the server sent only one version of NFS * to use for the DS. @@ -413,7 +374,7 @@ nfs4_ff_layout_prepare_ds(struct pnfs_layout_segment *lseg, u32 ds_idx, dataserver_retrans, mirror->mirror_ds->ds_versions[0].version, mirror->mirror_ds->ds_versions[0].minor_version, - flavor); + RPC_AUTH_UNIX); /* connect success, check rsize/wsize limit */ if (ds->ds_clp) { @@ -438,11 +399,7 @@ nfs4_ff_layout_prepare_ds(struct pnfs_layout_segment *lseg, u32 ds_idx, } else pnfs_error_mark_layout_for_return(ino, lseg); ds = NULL; - goto out; } -out_update_creds: - if (ff_layout_update_mirror_cred(mirror, ds)) - ds = NULL; out: return ds; }
We're just as likely to have allocation problems here as we would if we delay looking up the credential like we currently do. Fix the code to get a rpc_cred reference early, as soon as the mirror is set up. This allows us to eliminate the mirror early if there is a problem getting an rpc credential. This also allows us to drop the uid/gid from the layout_mirror struct as well. In the event that we find an existing mirror where this one would go, we swap in the new creds unconditionally, and drop the reference to the old one. Note that the old ff_layout_update_mirror_cred function wouldn't set this pointer unless the DS version was 3, but we don't know what the DS version is at this point. I'm a little unclear on why it did that as you still need creds to talk to v4 servers as well. I have the code set it regardless of the DS version here. Also note the change to using generic creds instead of calling lookup_cred directly. With that change, we also need to populate the group_info pointer in the acred as some functions expect that to never be NULL. Instead of allocating one every time however, we can allocate one when the module is loaded and share it since the group_info is refcounted. Signed-off-by: Jeff Layton <jeff.layton@primarydata.com> --- fs/nfs/flexfilelayout/flexfilelayout.c | 41 ++++++++++++++++++++++----- fs/nfs/flexfilelayout/flexfilelayout.h | 2 -- fs/nfs/flexfilelayout/flexfilelayoutdev.c | 47 ++----------------------------- 3 files changed, 36 insertions(+), 54 deletions(-)