pnfsd fixes
Subsystems (2)
cover pnfsd fixes
[PATCH 1/9] SQUASHME: pnfsd: fix lrs_present calculation [PATCH 2/9] SQUASHME: pnfsd: return_on_close all layout segments [PATCH 3/9] SQUASHME: pnfsd: do not dequeue layout state on [PATCH 4/9] SQUASHME: pnfsd: remove fs_layout_return sb parameter [PATCH 5/9] SQUASHME: pnfsd: fix locking around fs_layout_return [PATCH 6/9] pnfsd: LR_FLAG_CL_EMPTY layout return flag [PATCH 7/9] pnfsd: LR_FLAG_EMPTY layout return flag [PATCH 8/9] SQUASHME: pnfsd: Something very wrong with [PATCH 9/9] SQUASHME: pnfsd: no use for fi_layout_states list
PATCH 1/9 SQUASHME: pnfsd: fix lrs_present calculation
lrs_present must be set to zero iff the client holds no layout state on the respective file.
--- fs/nfsd/nfs4pnfsd.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-)
fs/nfsd/nfs4pnfsd.c +9 -6
@@ -893,7 +893,7 @@ struct super_block *struct nfs4_layout *lp, *nextlp;dprintk ("%s: clp %p fp %p\n" , __func__, clp, fp); - lrp->lrs_present =1 ; + lrp->lrs_present =0 ;spin_lock (&layout_lock); list_for_each_entry_safe (lp, nextlp, &fp->fi_layouts, lo_perfile) { dprintk("%s: lp %p client %p,%p lo_type %x,%x iomode %d,%d\n",
@@ -901,19 +901,22 @@ struct super_block * lp->lo_client, clp, lp->lo_seg.layout_type, lrp->args.lr_seg.layout_type, lp->lo_seg.iomode, lrp->args.lr_seg.iomode); -if (lp->lo_client != clp || - lp->lo_seg.layout_type != lrp->args.lr_seg.layout_type || +if (lp->lo_client != clp) +continue ; +if (lp->lo_seg.layout_type != lrp->args.lr_seg.layout_type || (lp->lo_seg.iomode != lrp->args.lr_seg.iomode && lrp->args.lr_seg.iomode !=IOMODE_ANY ) || - !lo_seg_overlapping (&lp->lo_seg, &lrp->args.lr_seg)) + !lo_seg_overlapping (&lp->lo_seg, &lrp->args.lr_seg)) { + lrp->lrs_present =1 ;continue ; + } layouts_found++;trim_layout (&lp->lo_seg, &lrp->args.lr_seg);if (!lp->lo_seg.length) { - lrp->lrs_present =0 ;dequeue_layout (lp);destroy_layout (lp); - } + }else + lrp->lrs_present =1 ; }if (ls && layouts_found && lrp->lrs_present)update_layout_stateid_locked (ls, &lrp->lr_sid);
Signature · 1 line
1.7.11.7
PATCH 2/9 SQUASHME: pnfsd: return_on_close all layout segments
In reply to Benny HalevyAs per errata #3226
http://www.rfc-editor.org/errata_search.php?rfc=5661return_on_close refers to all segments acquired by the client not only those marked with logr_return_on_close
--- fs/nfsd/nfs4pnfsd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
fs/nfsd/nfs4pnfsd.c +1 -1
@@ -1242,7 +1242,7 @@ void pnfsd_roc(struct nfs4_client *clp, struct nfs4_file *fp)bool empty;/* Check for a match */ -if (!lo->lo_state->ls_roc || lo->lo_client != clp) +if (lo->lo_client != clp)continue ;/* Return the layout */
Signature · 1 line
1.7.11.7
PATCH 3/9 SQUASHME: pnfsd: do not dequeue layout state on destroy_layout
In reply to Benny Halevythe layout state may be shared by a number of layout segments, each held by struct nfs4_layout and we may be destroying just one of them e.g. on layoutreturn.
--- fs/nfsd/nfs4pnfsd.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
fs/nfsd/nfs4pnfsd.c +2 -1
@@ -345,7 +345,6 @@ static void update_layout_roc(struct nfs4_layout_state *ls, bool roc) __func__, lp, clp, fp, fp->fi_inode);kmem_cache_free (pnfs_layout_slab, lp); -list_del_init (&ls->ls_perfile);/* release references taken by init_layout */ put_layout_state (ls);put_nfs4_file (fp);
@@ -1249,6 +1248,7 @@ void pnfsd_roc(struct nfs4_client *clp, struct nfs4_file *fp)memset (&lr,0 ,sizeof (lr)); lr.args.lr_return_type =RETURN_FILE ; lr.args.lr_seg = lo->lo_seg; +list_del_init (&lo->lo_state->ls_perfile);/* just to be on the safe side */ dequeue_layout (lo);destroy_layout (lo);/* do not access lp after this */
@@ -1301,6 +1301,7 @@ void pnfs_expire_client(struct nfs4_client *clp) lr.args.lr_seg = lp->lo_seg; empty =list_empty (&lp->lo_file->fi_layouts);BUG_ON (lp->lo_client != clp); +list_del_init (&lp->lo_state->ls_perfile);/* just to be on the safe side */ dequeue_layout (lp);destroy_layout (lp);/* do not access lp after this */ }
Signature · 1 line
1.7.11.7
PATCH 4/9 SQUASHME: pnfsd: remove fs_layout_return sb parameter
In reply to Benny HalevyFrom: Boaz Harrosh <[email protected]>
Derive the superblock from the inode argument. Let the caller provide the right inode, rather than NULL for RETURN_{FSID,ALL}
--- fs/nfsd/nfs4pnfsd.c | 25 ++++++++++--------------- 1 file changed, 10 insertions(+), 15 deletions(-)
fs/nfsd/nfs4pnfsd.c +10 -15
@@ -350,11 +350,12 @@ static void update_layout_roc(struct nfs4_layout_state *ls, bool roc)put_nfs4_file (fp); } -void fs_layout_return (struct super_block *sb,struct inode *ino, -struct nfsd4_pnfs_layoutreturn *lrp,int flags, -void *recall_cookie) +static void fs_layout_return (struct inode *ino, +struct nfsd4_pnfs_layoutreturn *lrp, +int flags,void *recall_cookie) {int ret; +struct super_block *sb = ino->i_sb;if (unlikely (!sb->s_pnfs_op->layout_return))return ;
@@ -362,9 +363,6 @@ void fs_layout_return(struct super_block *sb, struct inode *ino, lrp->args.lr_flags = flags; lrp->args.lr_cookie = recall_cookie; -if (!ino)/* FSID or ALL */ - ino = sb->s_root->d_inode; - ret = sb->s_pnfs_op->layout_return (ino, &lrp->args); dprintk("%s: inode %lu iomode=%d offset=0x%llx length=0x%llx " "cookie = %p flags 0x%x status=%d\n",
@@ -1081,7 +1079,7 @@ int nfs4_pnfs_return_layout(struct super_block *sb, struct svc_fh *current_fh,nfs4_unlock_state ();/* call exported filesystem layout_return (ignore return-code) */ -fs_layout_return (sb, ino, lrp,0 , recall_cookie); +fs_layout_return (ino, lrp,0 , recall_cookie); out_no_fs_call:dprintk ("pNFS %s: exit status %d\n" , __func__, status);
@@ -1201,9 +1199,8 @@ int nfs4_pnfs_return_layout(struct super_block *sb, struct svc_fh *current_fh, inode =igrab (clr->clr_file->fi_inode);if (WARN_ON (!inode))return ; - }else { - inode =NULL ; - } + }else + inode = clr->clr_sb->s_root->d_inode;dprintk ("%s: clp %p fp %p: simulating layout_return\n" , __func__, clr->clr_client, clr->clr_file);
@@ -1219,8 +1216,7 @@ int nfs4_pnfs_return_layout(struct super_block *sb, struct svc_fh *current_fh, recall_cookie =layoutrecall_done (clr);spin_unlock (&layout_lock); -fs_layout_return (clr->clr_sb, inode, &lr,LR_FLAG_INTERN , - recall_cookie); +fs_layout_return (inode, &lr,LR_FLAG_INTERN , recall_cookie);iput (inode); }
@@ -1255,8 +1251,7 @@ void pnfsd_roc(struct nfs4_client *clp, struct nfs4_file *fp) empty =list_empty (&fp->fi_layouts); found = true;dprintk ("%s: fp=%p clp=%p: return on close" , __func__, fp, clp); -fs_layout_return (fp->fi_inode->i_sb, fp->fi_inode, &lr, -LR_FLAG_INTERN , +fs_layout_return (fp->fi_inode, &lr,LR_FLAG_INTERN , empty ?PNFS_LAST_LAYOUT_NO_RECALLS :NULL ); }spin_unlock (&layout_lock);
@@ -1315,7 +1310,7 @@ void pnfs_expire_client(struct nfs4_client *clp)dprintk ("%s: inode %lu lp %p clp %p\n" , __func__, inode->i_ino, lp, clp); -fs_layout_return (inode->i_sb, inode, &lr,LR_FLAG_EXPIRE , +fs_layout_return (inode, &lr,LR_FLAG_EXPIRE , empty ?PNFS_LAST_LAYOUT_NO_RECALLS :NULL );iput (inode); }
Signature · 1 line
1.7.11.7
PATCH 5/9 SQUASHME: pnfsd: fix locking around fs_layout_return
In reply to Benny HalevyFrom: Boaz Harrosh <[email protected]>
Every code path that eventually called fs_layout_return had different locking. Some held both the layout_spinlock has well as nfs-lock. Some one or the other, some none. Fix all sites to never hold any locks, before calling The FS.
Also, deprecate PNFS_LAST_LAYOUT_NO_RECALLS.
To be replaced with lr_flags bit
--- fs/nfsd/nfs4pnfsd.c | 176 ++++++++++++++++++++++------------------ include/linux/nfsd/nfsd4_pnfs.h | 2 - 2 files changed, 97 insertions(+), 81 deletions(-)
fs/nfsd/nfs4pnfsd.c +97 -79
@@ -186,6 +186,9 @@ void pnfs_clear_device_notify(struct nfs4_client *clp)kfree (ls); } +/* +* Note: caller must not hold layout_lock +*/ static void put_layout_state (struct nfs4_layout_state *ls) {
@@ -321,6 +324,9 @@ static void update_layout_roc(struct nfs4_layout_state *ls, bool roc)dprintk ("pNFS %s end\n" , __func__); } +/* +* Note: always called under the layout_lock +*/ static void dequeue_layout (struct nfs4_layout *lp) {
@@ -329,7 +335,7 @@ static void update_layout_roc(struct nfs4_layout_state *ls, bool roc) }/* -* Note: always called under the layout_lock +* Note: caller must not hold layout_lock */ static void destroy_layout (struct nfs4_layout *lp)
@@ -881,10 +887,59 @@ struct super_block *dprintk ("%s:End lo %llu:%lld\n" , __func__, lo->offset, lo->length); } +static void +pnfsd_return_lo_list (struct list_head *lo_destroy_list,struct inode *ino_orig, +struct nfsd4_pnfs_layoutreturn *lr_orig,int flags, +void *cb_cookie) +{ +struct nfs4_layout *lo, *nextlp; + +if (list_empty (lo_destroy_list) && cb_cookie) { +/* This is a rare race case where at the time of recall there +* were some layouts, which got freed before the recall-done. +* and the recall was left without any actual layouts to free. +* If the FS gave us a cb_cookie it is waiting for it so we +* report back about it here. +*/ + +/* Any caller with a cb_cookie must pass a none null +* ino_orig and an lr_orig. +*/ +struct inode *inode =igrab (ino_orig); + +/* Probably a BUG_ON but just in case. Caller of cb_recall must +* take care of this. Please report to ml */ +if (WARN_ON (!inode)) +return ; + +fs_layout_return (inode, lr_orig, flags, cb_cookie); +iput (inode); +return ; + } + +list_for_each_entry_safe (lo, nextlp, lo_destroy_list, lo_perfile) { +struct inode *inode = lo->lo_file->fi_inode; +struct nfsd4_pnfs_layoutreturn lr; +bool empty; + +memset (&lr,0 ,sizeof (lr)); + lr.args.lr_return_type =RETURN_FILE ; + lr.args.lr_seg = lo->lo_seg; + +list_del (&lo->lo_perfile); + empty =list_empty (lo_destroy_list); + +fs_layout_return (inode, &lr, flags, empty ? cb_cookie :NULL ); + +destroy_layout (lo);/* this will put the lo_file */ + } +} +static int pnfs_return_file_layouts (struct nfs4_client *clp,struct nfs4_file *fp,struct nfsd4_pnfs_layoutreturn *lrp, -struct nfs4_layout_state *ls) +struct nfs4_layout_state *ls, +struct list_head *lo_destroy_list) {int layouts_found =0 ;struct nfs4_layout *lp, *nextlp;
@@ -911,7 +966,7 @@ struct super_block *trim_layout (&lp->lo_seg, &lrp->args.lr_seg);if (!lp->lo_seg.length) {dequeue_layout (lp); -destroy_layout (lp); +list_add_tail (&lp->lo_perfile, lo_destroy_list); }else lrp->lrs_present =1 ; }
@@ -924,7 +979,8 @@ struct super_block *static int pnfs_return_client_layouts (struct nfs4_client *clp, -struct nfsd4_pnfs_layoutreturn *lrp,u64 ex_fsid) +struct nfsd4_pnfs_layoutreturn *lrp,u64 ex_fsid, +struct list_head *lo_destroy_list) {int layouts_found =0 ;struct nfs4_layout *lp, *nextlp;
@@ -942,7 +998,7 @@ struct super_block * layouts_found++;dequeue_layout (lp); -destroy_layout (lp); +list_add_tail (&lp->lo_perfile, lo_destroy_list); }spin_unlock (&layout_lock);
@@ -1004,10 +1060,10 @@ int nfs4_pnfs_return_layout(struct super_block *sb, struct svc_fh *current_fh,struct inode *ino = current_fh->fh_dentry->d_inode;struct nfs4_file *fp =NULL ;struct nfs4_client *clp; -struct nfs4_layout_state *ls =NULL ;struct nfs4_layoutrecall *clr, *nextclr;u64 ex_fsid = current_fh->fh_export->ex_fsid;void *recall_cookie =NULL ; +LIST_HEAD (lo_destroy_list);dprintk ("NFSD: %s\n" , __func__);
@@ -1017,6 +1073,8 @@ int nfs4_pnfs_return_layout(struct super_block *sb, struct svc_fh *current_fh, goto out;if (lrp->args.lr_return_type ==RETURN_FILE ) { +struct nfs4_layout_state *ls =NULL ; + fp =find_file (ino);if (!fp) {nfs4_unlock_state ();
@@ -1037,12 +1095,12 @@ int nfs4_pnfs_return_layout(struct super_block *sb, struct svc_fh *current_fh, goto out_put_file;/* update layouts */ - layouts_found =pnfs_return_file_layouts (clp, fp, lrp, ls); -/* optimize for the all-empty case */ -if (list_empty (&fp->fi_layouts)) - recall_cookie =PNFS_LAST_LAYOUT_NO_RECALLS ; + layouts_found =pnfs_return_file_layouts (clp, fp, lrp, ls, + &lo_destroy_list); +put_layout_state (ls); }else { - layouts_found =pnfs_return_client_layouts (clp, lrp, ex_fsid); + layouts_found =pnfs_return_client_layouts (clp, lrp, ex_fsid, + &lo_destroy_list); } dprintk("pNFS %s: clp %p fp %p layout_type 0x%x iomode %d "
@@ -1073,13 +1131,12 @@ int nfs4_pnfs_return_layout(struct super_block *sb, struct svc_fh *current_fh, out_put_file:if (fp)put_nfs4_file (fp); -if (ls) -put_layout_state (ls); + out:nfs4_unlock_state (); -/* call exported filesystem layout_return (ignore return-code) */ -fs_layout_return (ino, lrp,0 , recall_cookie); +pnfsd_return_lo_list (&lo_destroy_list, ino ? ino : sb->s_root->d_inode, + lrp,0 , recall_cookie); out_no_fs_call:dprintk ("pNFS %s: exit status %d\n" , __func__, status);
@@ -1194,30 +1251,26 @@ int nfs4_pnfs_return_layout(struct super_block *sb, struct svc_fh *current_fh, };struct inode *inode;void *recall_cookie; - -if (clr->clr_file) { - inode =igrab (clr->clr_file->fi_inode); -if (WARN_ON (!inode)) -return ; - }else - inode = clr->clr_sb->s_root->d_inode; +LIST_HEAD (lo_destroy_list);dprintk ("%s: clp %p fp %p: simulating layout_return\n" , __func__, clr->clr_client, clr->clr_file);if (clr->cb.cbl_recall_type ==RETURN_FILE )pnfs_return_file_layouts (clr->clr_client, clr->clr_file, &lr, -NULL ); +NULL , &lo_destroy_list);else pnfs_return_client_layouts (clr->clr_client, &lr, - clr->cb.cbl_fsid.major); + clr->cb.cbl_fsid.major, + &lo_destroy_list);spin_lock (&layout_lock); recall_cookie =layoutrecall_done (clr);spin_unlock (&layout_lock); -fs_layout_return (inode, &lr,LR_FLAG_INTERN , recall_cookie); -iput (inode); + inode = clr->clr_file->fi_inode ?: clr->clr_sb->s_root->d_inode; +pnfsd_return_lo_list (&lo_destroy_list, inode, &lr,LR_FLAG_INTERN , + recall_cookie); }/* Return On Close:
@@ -1228,39 +1281,31 @@ int nfs4_pnfs_return_layout(struct super_block *sb, struct svc_fh *current_fh,void pnfsd_roc (struct nfs4_client *clp,struct nfs4_file *fp) {struct nfs4_layout *lo, *nextlp; -bool found = false; +LIST_HEAD (lo_destroy_list); +/* TODO: We need to also free layout recalls like pnfs_expire_client */ dprintk ("%s: fp=%p clp=%p" , __func__, fp, clp);spin_lock (&layout_lock); list_for_each_entry_safe (lo, nextlp, &fp->fi_layouts, lo_perfile) { -struct nfsd4_pnfs_layoutreturn lr; -bool empty; -/* Check for a match */ if (lo->lo_client != clp)continue ;/* Return the layout */ -memset (&lr,0 ,sizeof (lr)); - lr.args.lr_return_type =RETURN_FILE ; - lr.args.lr_seg = lo->lo_seg;list_del_init (&lo->lo_state->ls_perfile);/* just to be on the safe side */ dequeue_layout (lo); -destroy_layout (lo);/* do not access lp after this */ - - empty =list_empty (&fp->fi_layouts); - found = true; -dprintk ("%s: fp=%p clp=%p: return on close" , __func__, fp, clp); -fs_layout_return (fp->fi_inode, &lr,LR_FLAG_INTERN , - empty ?PNFS_LAST_LAYOUT_NO_RECALLS :NULL ); +list_add_tail (&lo->lo_perfile, &lo_destroy_list); }spin_unlock (&layout_lock); -if (!found) -dprintk ("%s: no layout found" , __func__); + +pnfsd_return_lo_list (&lo_destroy_list,NULL ,NULL ,LR_FLAG_INTERN ,NULL ); }void pnfs_expire_client (struct nfs4_client *clp) { +struct nfs4_layout *lo, *nextlo; +LIST_HEAD (lo_destroy_list); +for (;;) {struct nfs4_layoutrecall *lrp =NULL ;
@@ -1280,40 +1325,17 @@ void pnfs_expire_client(struct nfs4_client *clp)put_layoutrecall (lrp); } -for (;;) { -struct nfs4_layout *lp =NULL ; -struct inode *inode =NULL ; -struct nfsd4_pnfs_layoutreturn lr; -bool empty = false; - -spin_lock (&layout_lock); -if (!list_empty (&clp->cl_layouts)) { - lp =list_entry (clp->cl_layouts.next, - struct nfs4_layout, lo_perclnt); - inode =igrab (lp->lo_file->fi_inode); -memset (&lr,0 ,sizeof (lr)); - lr.args.lr_return_type =RETURN_FILE ; - lr.args.lr_seg = lp->lo_seg; - empty =list_empty (&lp->lo_file->fi_layouts); -BUG_ON (lp->lo_client != clp); -list_del_init (&lp->lo_state->ls_perfile);/* just to be on the safe side */ -dequeue_layout (lp); -destroy_layout (lp);/* do not access lp after this */ - } -spin_unlock (&layout_lock); -if (!lp) -break ; - -if (WARN_ON (!inode)) -break ; - -dprintk ("%s: inode %lu lp %p clp %p\n" , __func__, inode->i_ino, - lp, clp); - -fs_layout_return (inode, &lr,LR_FLAG_EXPIRE , - empty ?PNFS_LAST_LAYOUT_NO_RECALLS :NULL ); -iput (inode); +spin_lock (&layout_lock); + list_for_each_entry_safe (lo, nextlo, &clp->cl_layouts, lo_perclnt) { +BUG_ON (lo->lo_client != clp); +dequeue_layout (lo); +list_add_tail (&lo->lo_perfile, &lo_destroy_list); +dprintk ("%s: inode %lu lp %p clp %p\n" , __func__, + lo->lo_file->fi_inode->i_ino, lo, clp); } +spin_unlock (&layout_lock); + +pnfsd_return_lo_list (&lo_destroy_list,NULL ,NULL ,LR_FLAG_EXPIRE ,NULL ); }struct create_recall_list_arg {
@@ -1516,10 +1538,6 @@ struct create_recall_list_arg {INIT_LIST_HEAD (&todolist); -/* If no cookie provided by FS, return a default one */ -if (!cbl->cbl_cookie) - cbl->cbl_cookie =PNFS_LAST_LAYOUT_NO_RECALLS ; - status =create_layout_recall_list (&todolist, &todo_len, cbl, lrfile);if (list_empty (&todolist)) { status = -ENOENT ;
include/linux/nfsd/nfsd4_pnfs.h +0 -2
@@ -110,8 +110,6 @@ struct nfsd4_pnfs_layoutcommit_res {u64 lc_newsize;/* response */ }; -#define PNFS_LAST_LAYOUT_NO_RECALLS ((void *)-1)/* used with lr_cookie below */ -enum layoutreturn_flags {LR_FLAG_INTERN =1 <<0 ,/* internal return */ LR_FLAG_EXPIRE =1 <<1 ,/* return on client expiration */
Signature · 1 line
1.7.11.7
PATCH 6/9 pnfsd: LR_FLAG_CL_EMPTY layout return flag
In reply to Benny Halevy--- fs/nfsd/nfs4pnfsd.c | 17 +++++++++++++---- include/linux/nfsd/nfsd4_pnfs.h | 1 + 2 files changed, 14 insertions(+), 4 deletions(-)
fs/nfsd/nfs4pnfsd.c +13 -4
@@ -1064,6 +1064,7 @@ int nfs4_pnfs_return_layout(struct super_block *sb, struct svc_fh *current_fh,u64 ex_fsid = current_fh->fh_export->ex_fsid;void *recall_cookie =NULL ;LIST_HEAD (lo_destroy_list); +int lr_flags =0 ;dprintk ("NFSD: %s\n" , __func__);
@@ -1098,9 +1099,12 @@ int nfs4_pnfs_return_layout(struct super_block *sb, struct svc_fh *current_fh, layouts_found =pnfs_return_file_layouts (clp, fp, lrp, ls, &lo_destroy_list);put_layout_state (ls); +if (!lrp->lrs_present) + lr_flags |=LR_FLAG_CL_EMPTY ; }else { layouts_found =pnfs_return_client_layouts (clp, lrp, ex_fsid, &lo_destroy_list); + lr_flags |=LR_FLAG_CL_EMPTY ; } dprintk("pNFS %s: clp %p fp %p layout_type 0x%x iomode %d "
@@ -1136,7 +1140,7 @@ int nfs4_pnfs_return_layout(struct super_block *sb, struct svc_fh *current_fh,nfs4_unlock_state ();pnfsd_return_lo_list (&lo_destroy_list, ino ? ino : sb->s_root->d_inode, - lrp,0 , recall_cookie); + lrp, lr_flags, recall_cookie); out_no_fs_call:dprintk ("pNFS %s: exit status %d\n" , __func__, status);
@@ -1252,24 +1256,29 @@ int nfs4_pnfs_return_layout(struct super_block *sb, struct svc_fh *current_fh,struct inode *inode;void *recall_cookie;LIST_HEAD (lo_destroy_list); +int lr_flags =LR_FLAG_INTERN ;dprintk ("%s: clp %p fp %p: simulating layout_return\n" , __func__, clr->clr_client, clr->clr_file); -if (clr->cb.cbl_recall_type ==RETURN_FILE ) +if (clr->cb.cbl_recall_type ==RETURN_FILE ) {pnfs_return_file_layouts (clr->clr_client, clr->clr_file, &lr,NULL , &lo_destroy_list); -else +if (!lr.lrs_present) + lr_flags |=LR_FLAG_CL_EMPTY ; + }else {pnfs_return_client_layouts (clr->clr_client, &lr, clr->cb.cbl_fsid.major, &lo_destroy_list); + lr_flags |=LR_FLAG_CL_EMPTY ; + }spin_lock (&layout_lock); recall_cookie =layoutrecall_done (clr);spin_unlock (&layout_lock); inode = clr->clr_file->fi_inode ?: clr->clr_sb->s_root->d_inode; -pnfsd_return_lo_list (&lo_destroy_list, inode, &lr,LR_FLAG_INTERN , +pnfsd_return_lo_list (&lo_destroy_list, inode, &lr, lr_flags, recall_cookie); }
include/linux/nfsd/nfsd4_pnfs.h +1 -0
@@ -113,6 +113,7 @@ struct nfsd4_pnfs_layoutcommit_res {enum layoutreturn_flags {LR_FLAG_INTERN =1 <<0 ,/* internal return */ LR_FLAG_EXPIRE =1 <<1 ,/* return on client expiration */ +LR_FLAG_CL_EMPTY =1 <<2 ,/* No more layout for returning client */ };struct nfsd4_pnfs_layoutreturn_arg {
Signature · 1 line
1.7.11.7
PATCH 7/9 pnfsd: LR_FLAG_EMPTY layout return flag
In reply to Benny Halevy--- fs/nfsd/nfs4pnfsd.c | 8 +++++++- include/linux/nfsd/nfsd4_pnfs.h | 1 + 2 files changed, 8 insertions(+), 1 deletion(-)
fs/nfsd/nfs4pnfsd.c +7 -1
@@ -921,15 +921,21 @@ struct super_block *struct inode *inode = lo->lo_file->fi_inode;struct nfsd4_pnfs_layoutreturn lr;bool empty; +int lr_flags = flags;memset (&lr,0 ,sizeof (lr)); lr.args.lr_return_type =RETURN_FILE ; lr.args.lr_seg = lo->lo_seg; +spin_lock (&layout_lock); +if (list_empty (&lo->lo_file->fi_layouts)) + lr_flags |=LR_FLAG_EMPTY ; +spin_unlock (&layout_lock); +list_del (&lo->lo_perfile); empty =list_empty (lo_destroy_list); -fs_layout_return (inode, &lr, flags, empty ? cb_cookie :NULL ); +fs_layout_return (inode, &lr, lr_flags, empty ? cb_cookie :NULL );destroy_layout (lo);/* this will put the lo_file */ }
include/linux/nfsd/nfsd4_pnfs.h +1 -0
@@ -114,6 +114,7 @@ enum layoutreturn_flags {LR_FLAG_INTERN =1 <<0 ,/* internal return */ LR_FLAG_EXPIRE =1 <<1 ,/* return on client expiration */ LR_FLAG_CL_EMPTY =1 <<2 ,/* No more layout for returning client */ + LR_FLAG_EMPTY = 1 << 3, /* No more layout for file */ }; struct nfsd4_pnfs_layoutreturn_arg {
Signature · 1 line
1.7.11.7
PATCH 8/9 SQUASHME: pnfsd: Something very wrong with layout_recall(RETURN_FILE)
In reply to Benny HalevyFrom: Boaz Harrosh <[email protected]>
In patch:
pnfsd: layout recall layout state
the cl_has_file_layout() is no longer inspecting the layout structures added per file but is inspecting if file has layout_state.
So it is counting layout_states and not layouts
This is bad because the addition of the layout_states on the file is done before the call to the filesystem so if the FS does a recall, the nfsd is confused thinking it already has a layout and issues a recall. Instead of returning -ENOENT, ie list is empty. The client then truly returns nomaching_layout and when the lo_return(s) are emulated the system gets stuck is some reference miss-match. (UML so no crash trace)
Now lets say that the state should be set before the call to the FS. Then I don't see where the state is removed in the case of an ERROR return from FS->layout_get. Meaning cl_has_file_layout() will always think it has some count.
Also When a layout is returned it is the layout list that is inspected and freed, so how is the cl_has_file_layout() emptied ?
In any way. I do not agree that it is the state that is needed to be searched in cl_has_file_layout() but it is layouts that are needed, otherwise the all layout <---> recall very delicate dance is totally broken.
What was the meaning of the Poet?
I reverted the cl_has_file_layout() to historical processing.
Also cl_has_file_layout() returns true for any layout on a file, but we must inspect IO_MODE and LSEG for a partial-match, as well.
The below works for me. State also looks good. I can now safely call cb_recall, from within a layout_get operation.
--- fs/nfsd/nfs4pnfsd.c | 29 ++++++++++++++++------------- 1 file changed, 16 insertions(+), 13 deletions(-)
fs/nfsd/nfs4pnfsd.c +16 -13
@@ -1192,24 +1192,27 @@ int nfs4_pnfs_return_layout(struct super_block *sb, struct svc_fh *current_fh, }static bool -cl_has_file_layout (struct nfs4_client *clp,struct nfs4_file *fp,stateid_t *lsid) +cl_has_file_layout (struct nfs4_client *clp,struct nfs4_file *fp, +stateid_t *lsid,struct nfsd4_pnfs_cb_layout *cbl) { -struct nfs4_layout_state *ls; +struct nfs4_layout *lo; +bool ret = false;spin_lock (&layout_lock); - list_for_each_entry (ls, &fp->fi_layout_states, ls_perfile) -if (same_clid (&ls->ls_stid.sc_stateid.si_opaque.so_clid, - &clp->cl_clientid)) { +list_for_each_entry (lo, &fp->fi_layouts, lo_perfile) { +if (same_clid (&lo->lo_client->cl_clientid, &clp->cl_clientid) && +lo_seg_overlapping (&cbl->cbl_seg, &lo->lo_seg) && + (cbl->cbl_seg.iomode & lo->lo_seg.iomode)) goto found; - } -spin_unlock (&layout_lock); -return false; - + } + goto unlock; found: -update_layout_stateid_locked (ls, lsid); +/* Im going to send a recall on this latout update state */ +update_layout_stateid_locked (lo->lo_state, lsid); + ret = true; +unlock:spin_unlock (&layout_lock); - -return true; +return ret; } static int
@@ -1241,7 +1244,7 @@ int nfs4_pnfs_return_layout(struct super_block *sb, struct svc_fh *current_fh, {switch (cbl->cbl_recall_type) {case RETURN_FILE : -return cl_has_file_layout (clp, lrfile, lsid); +return cl_has_file_layout (clp, lrfile, lsid, cbl);case RETURN_FSID :return cl_has_fsid_layout (clp, &cbl->cbl_fsid);default :
Signature · 1 line
1.7.11.7
PATCH 9/9 SQUASHME: pnfsd: no use for fi_layout_states list anymore
In reply to Benny Halevy--- fs/nfsd/nfs4pnfsd.c | 10 ---------- fs/nfsd/nfs4state.c | 1 - fs/nfsd/pnfsd.h | 1 - fs/nfsd/state.h | 1 - 4 files changed, 13 deletions(-)
fs/nfsd/nfs4pnfsd.c +0 -10
@@ -157,10 +157,6 @@ void pnfs_clear_device_notify(struct nfs4_client *clp)return new;kref_init (&new->ls_ref);nfsd4_init_stid (&new->ls_stid, clp,NFS4_LAYOUT_STID ); -INIT_LIST_HEAD (&new->ls_perfile); -spin_lock (&layout_lock); -list_add (&new->ls_perfile, &fp->fi_layout_states); -spin_unlock (&layout_lock); new->ls_roc = false;return new; }
@@ -178,11 +174,6 @@ void pnfs_clear_device_notify(struct nfs4_client *clp) container_of(kref, struct nfs4_layout_state, ls_ref);nfsd4_unhash_stid (&ls->ls_stid); -if (!list_empty (&ls->ls_perfile)) { -spin_lock (&layout_lock); -list_del (&ls->ls_perfile); -spin_unlock (&layout_lock); - }kfree (ls); }
@@ -1310,7 +1301,6 @@ void pnfsd_roc(struct nfs4_client *clp, struct nfs4_file *fp)continue ;/* Return the layout */ -list_del_init (&lo->lo_state->ls_perfile);/* just to be on the safe side */ dequeue_layout (lo);list_add_tail (&lo->lo_perfile, &lo_destroy_list); }
fs/nfsd/nfs4state.c +0 -1
@@ -2370,7 +2370,6 @@ static void nfsd4_init_file(struct nfs4_file *fp, struct inode *ino,memset (fp->fi_access,0 ,sizeof (fp->fi_access));#if defined(CONFIG_PNFSD )INIT_LIST_HEAD (&fp->fi_layouts); -INIT_LIST_HEAD (&fp->fi_layout_states); fp->fi_fsid.major = current_fh->fh_export->ex_fsid; fp->fi_fsid.minor =0 ; fp->fi_fhlen = current_fh->fh_handle.fh_size;
fs/nfsd/pnfsd.h +0 -1
@@ -44,7 +44,6 @@struct nfs4_layout_state {struct nfs4_stid ls_stid;/* must be first field */ struct kref ls_ref; -struct list_head ls_perfile;bool ls_roc; };
fs/nfsd/state.h +0 -1
@@ -399,7 +399,6 @@ struct nfs4_file {bool fi_had_conflict;#if defined(CONFIG_PNFSD )struct list_head fi_layouts; -struct list_head fi_layout_states;/* used by layoutget / layoutrecall */ struct nfs4_fsid fi_fsid;u32 fi_fhlen;
Signature · 1 line
1.7.11.7