* [PATCH 1/4] pnfs fixes
@ 2010-10-27 18:21 Benny Halevy
2010-10-27 18:22 ` [PATCH 1/1] SQUASHME: pnfsd-files: update layout stateid properly Benny Halevy
` (3 more replies)
0 siblings, 4 replies; 14+ messages in thread
From: Benny Halevy @ 2010-10-27 18:21 UTC (permalink / raw)
To: NFS list
Here are some pnfs fixes for 2.6.36 since the bakeathon:
[PATCH 1/1] SQUASHME: pnfsd-files: update layout stateid properly
[PATCH 1/1] pnfs: do not change layout stateid when dropping layouts.
[PATCH 2/3] pnfs: mark page with error in readpage_async_filler when crossing lsegs
[PATCH 3/3] SQUASHME: pnfs: filelayout: print_ds should use dprintk
^ permalink raw reply [flat|nested] 14+ messages in thread* [PATCH 1/1] SQUASHME: pnfsd-files: update layout stateid properly 2010-10-27 18:21 [PATCH 1/4] pnfs fixes Benny Halevy @ 2010-10-27 18:22 ` Benny Halevy 2010-10-27 18:23 ` [PATCH 1/1] pnfs: do not change layout stateid when dropping layouts Benny Halevy ` (2 subsequent siblings) 3 siblings, 0 replies; 14+ messages in thread From: Benny Halevy @ 2010-10-27 18:22 UTC (permalink / raw) To: linux-nfs From: Benny Halevy <bhalevy@compute-1-5.palab.panasas.com> the layout stateid must be updated by layout get only on success upon changing the actual state, otherwise, a parallel layout_recall will send the wrong stateid. Signed-off-by: Benny Halevy <bhalevy@panasas.com> --- fs/nfsd/nfs4pnfsd.c | 35 ++++++++++++++++++++++------------- 1 files changed, 22 insertions(+), 13 deletions(-) diff --git a/fs/nfsd/nfs4pnfsd.c b/fs/nfsd/nfs4pnfsd.c index 9ce2bf2..fc7f4e5 100644 --- a/fs/nfsd/nfs4pnfsd.c +++ b/fs/nfsd/nfs4pnfsd.c @@ -302,7 +302,7 @@ nfs4_process_layout_stateid(struct nfs4_client *clp, struct nfs4_file *fp, if (lsp && ((verify_stateid(fp, stateid)) == 0)) { dprintk("%s parallel initial layout state\n", __func__); - goto update; + goto verified; } dprintk("%s ERROR bad opaque in stateid 1\n", __func__); @@ -314,14 +314,9 @@ nfs4_process_layout_stateid(struct nfs4_client *clp, struct nfs4_file *fp, dprintk("%s bad stateid 1\n", __func__); goto out_put; } -update: - update_stateid(&ls->ls_stateid); - dprintk("%s Updated ls_stateid to %d on layoutstate %p\n", - __func__, ls->ls_stateid.si_generation, ls); } +verified: status = 0; - /* Set the stateid to be encoded */ - memcpy(stateid, &ls->ls_stateid, sizeof(stateid_t)); /* Return the layout state if requested */ if (lsp) { @@ -351,13 +346,21 @@ free_layout(struct nfs4_layout *lp) kmem_cache_free(pnfs_layout_slab, lp); } +#define update_layout_stateid(ls, sid) { \ + update_stateid(&(ls)->ls_stateid); \ + dprintk("%s Updated ls_stateid to %d on layoutstate %p\n", \ + __func__, (ls)->ls_stateid.si_generation, (ls)); \ + memcpy((sid), &(ls)->ls_stateid, sizeof(stateid_t)); \ +} + static void init_layout(struct nfs4_layout_state *ls, struct nfs4_layout *lp, struct nfs4_file *fp, struct nfs4_client *clp, struct svc_fh *current_fh, - struct nfsd4_layout_seg *seg) + struct nfsd4_layout_seg *seg, + stateid_t *stateid) { dprintk("pNFS %s: ls %p lp %p clp %p fp %p ino %p\n", __func__, ls, lp, clp, fp, fp->fi_inode); @@ -369,6 +372,7 @@ init_layout(struct nfs4_layout_state *ls, lp->lo_state = ls; memcpy(&lp->lo_seg, seg, sizeof(lp->lo_seg)); spin_lock(&layout_lock); + update_layout_stateid(ls, stateid); list_add_tail(&lp->lo_perstate, &ls->ls_layouts); list_add_tail(&lp->lo_perclnt, &clp->cl_layouts); list_add_tail(&lp->lo_perfile, &fp->fi_layouts); @@ -717,7 +721,7 @@ nfs4_pnfs_get_layout(struct nfsd4_pnfs_layoutget *lgp, goto out_freelayout; /* Can't merge, so let's initialize this new layout */ - init_layout(ls, lp, fp, clp, lgp->lg_fhp, &res.lg_seg); + init_layout(ls, lp, fp, clp, lgp->lg_fhp, &res.lg_seg, &lgp->lg_sid); out_unlock: if (ls) put_layout_state(ls); @@ -773,7 +777,8 @@ out: static int pnfs_return_file_layouts(struct nfs4_client *clp, struct nfs4_file *fp, - struct nfsd4_pnfs_layoutreturn *lrp) + struct nfsd4_pnfs_layoutreturn *lrp, + struct nfs4_layout_state *ls) { int layouts_found = 0; struct nfs4_layout *lp, *nextlp; @@ -799,6 +804,8 @@ pnfs_return_file_layouts(struct nfs4_client *clp, struct nfs4_file *fp, destroy_layout(lp); } } + if (ls && layouts_found && lrp->lrs_present) + update_layout_stateid(ls, &lrp->lr_sid); spin_unlock(&layout_lock); return layouts_found; @@ -838,6 +845,7 @@ 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; u64 ex_fsid = current_fh->fh_export->ex_fsid; void *recall_cookie = NULL; @@ -859,13 +867,12 @@ int nfs4_pnfs_return_layout(struct super_block *sb, struct svc_fh *current_fh, /* Check the stateid */ dprintk("%s PROCESS LO_STATEID inode %p\n", __func__, ino); - status = nfs4_process_layout_stateid(clp, fp, &lrp->lr_sid, - NULL); + status = nfs4_process_layout_stateid(clp, fp, &lrp->lr_sid, &ls); if (status) goto out_put_file; /* update layouts */ - layouts_found = pnfs_return_file_layouts(clp, fp, lrp); + 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; @@ -884,6 +891,8 @@ 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(); -- 1.7.2.3 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 1/1] pnfs: do not change layout stateid when dropping layouts. 2010-10-27 18:21 [PATCH 1/4] pnfs fixes Benny Halevy 2010-10-27 18:22 ` [PATCH 1/1] SQUASHME: pnfsd-files: update layout stateid properly Benny Halevy @ 2010-10-27 18:23 ` Benny Halevy 2010-10-27 18:24 ` [PATCH 2/3] pnfs: mark page with error in readpage_async_filler when crossing lsegs Benny Halevy 2010-10-27 18:24 ` [PATCH 3/3] SQUASHME: pnfs: filelayout: print_ds should use dprintk Benny Halevy 3 siblings, 0 replies; 14+ messages in thread From: Benny Halevy @ 2010-10-27 18:23 UTC (permalink / raw) To: linux-nfs On voluntary "forgets", where the client drops the layout on its own the inode's layotu stateid must not change. Signed-off-by: Benny Halevy <bhalevy@panasas.com> --- fs/nfs/nfs4xdr.c | 1 + fs/nfs/pnfs.c | 6 ++++-- include/linux/nfs_xdr.h | 1 + 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c index 6d86633..31ccacf 100644 --- a/fs/nfs/nfs4xdr.c +++ b/fs/nfs/nfs4xdr.c @@ -5243,6 +5243,7 @@ static int decode_layoutreturn(struct xdr_stream *xdr, p = xdr_inline_decode(xdr, 4); if (unlikely(!p)) goto out_overflow; + res->valid = true; res->lrs_present = be32_to_cpup(p); if (res->lrs_present) status = decode_stateid(xdr, &res->stateid); diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c index ff1749a..8fa4887 100644 --- a/fs/nfs/pnfs.c +++ b/fs/nfs/pnfs.c @@ -593,9 +593,11 @@ pnfs_layoutreturn_release(struct nfs4_layoutreturn *lrp) return; spin_lock(&lrp->args.inode->i_lock); pnfs_clear_lseg_list(lo, &tmp_list, &lrp->args.range); - if (!lrp->res.lrs_present) + if (!lrp->res.valid) + ; /* forgetful model internal release */ + else if (!lrp->res.lrs_present) pnfs_invalidate_layout_stateid(lo); - else + else pnfs_set_layout_stateid(lo, &lrp->res.stateid); put_layout_hdr_locked(lo); /* Matched in _pnfs_return_layout */ spin_unlock(&lrp->args.inode->i_lock); diff --git a/include/linux/nfs_xdr.h b/include/linux/nfs_xdr.h index 80e6a36..37ff78a 100644 --- a/include/linux/nfs_xdr.h +++ b/include/linux/nfs_xdr.h @@ -269,6 +269,7 @@ struct nfs4_layoutreturn_args { struct nfs4_layoutreturn_res { struct nfs4_sequence_res seq_res; + bool valid; /* internal, true if received reply */ u32 lrs_present; nfs4_stateid stateid; }; -- 1.7.2.3 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 2/3] pnfs: mark page with error in readpage_async_filler when crossing lsegs 2010-10-27 18:21 [PATCH 1/4] pnfs fixes Benny Halevy 2010-10-27 18:22 ` [PATCH 1/1] SQUASHME: pnfsd-files: update layout stateid properly Benny Halevy 2010-10-27 18:23 ` [PATCH 1/1] pnfs: do not change layout stateid when dropping layouts Benny Halevy @ 2010-10-27 18:24 ` Benny Halevy 2010-10-27 18:24 ` [PATCH 3/3] SQUASHME: pnfs: filelayout: print_ds should use dprintk Benny Halevy 3 siblings, 0 replies; 14+ messages in thread From: Benny Halevy @ 2010-10-27 18:24 UTC (permalink / raw) To: linux-nfs From: Benny Halevy <bhalevy@compute-1-5.palab.panasas.com> Signed-off-by: Benny Halevy <bhalevy@compute-1-5.palab.panasas.com> Signed-off-by: Benny Halevy <bhalevy@panasas.com> --- fs/nfs/read.c | 20 ++++++++++---------- 1 files changed, 10 insertions(+), 10 deletions(-) diff --git a/fs/nfs/read.c b/fs/nfs/read.c index e1e1a65..1df536a 100644 --- a/fs/nfs/read.c +++ b/fs/nfs/read.c @@ -605,24 +605,24 @@ readpage_async_filler(void *data, struct page *page) { struct nfs_readdesc *desc = (struct nfs_readdesc *)data; struct inode *inode = page->mapping->host; - struct pnfs_layout_range *range; struct nfs_page *new; unsigned int len; - loff_t pgoff; int error; len = nfs_page_length(page); if (len == 0) return nfs_return_empty_page(page); - pgoff = (loff_t)page->index << PAGE_CACHE_SHIFT; - range = desc->pgio->pg_lseg ? &desc->pgio->pg_lseg->range : NULL; - if (!range || - (range->offset > pgoff + len) || - (range->offset + range->length < pgoff)) { - put_lseg(desc->pgio->pg_lseg); - desc->pgio->pg_lseg = pnfs_update_layout(inode, desc->ctx, - pgoff, len, IOMODE_READ); + if (desc->pgio->pg_lseg) { + loff_t pgoff = (loff_t)page->index << PAGE_CACHE_SHIFT; + struct pnfs_layout_range *range = &desc->pgio->pg_lseg->range; + + /* retry later with the right lseg? */ + if (range->offset > pgoff + len || + range->offset + range->length < pgoff) { + new = ERR_PTR(-EAGAIN); + goto out_error; + } } new = nfs_create_request(desc->ctx, inode, page, 0, len, -- 1.7.2.3 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* [PATCH 3/3] SQUASHME: pnfs: filelayout: print_ds should use dprintk 2010-10-27 18:21 [PATCH 1/4] pnfs fixes Benny Halevy ` (2 preceding siblings ...) 2010-10-27 18:24 ` [PATCH 2/3] pnfs: mark page with error in readpage_async_filler when crossing lsegs Benny Halevy @ 2010-10-27 18:24 ` Benny Halevy 2010-10-27 19:49 ` Fred Isaman 3 siblings, 1 reply; 14+ messages in thread From: Benny Halevy @ 2010-10-27 18:24 UTC (permalink / raw) To: linux-nfs rather than printk to prevent printouts in non-debug mode currently happening in filelayout_commit Signed-off-by: Benny Halevy <bhalevy@panasas.com> --- fs/nfs/nfs4filelayoutdev.c | 9 ++++----- 1 files changed, 4 insertions(+), 5 deletions(-) diff --git a/fs/nfs/nfs4filelayoutdev.c b/fs/nfs/nfs4filelayoutdev.c index 1f0ab62..de47112 100644 --- a/fs/nfs/nfs4filelayoutdev.c +++ b/fs/nfs/nfs4filelayoutdev.c @@ -53,10 +53,10 @@ void print_ds(struct nfs4_pnfs_ds *ds) { if (ds == NULL) { - printk("%s NULL device\n", __func__); + dprintk("%s NULL device\n", __func__); return; } - printk(" ip_addr %x port %hu\n" + dprintk(" ip_addr %x port %hu\n" " ref count %d\n" " client %p\n" " cl_exchange_flags %x\n", @@ -71,7 +71,7 @@ print_ds_list(struct nfs4_file_layout_dsaddr *dsaddr) int i; ifdebug(FACILITY) { - printk("%s dsaddr->ds_num %d\n", __func__, + dprintk("%s dsaddr->ds_num %d\n", __func__, dsaddr->ds_num); for (i = 0; i < dsaddr->ds_num; i++) print_ds(dsaddr->ds_list[i]); @@ -211,8 +211,7 @@ static void destroy_ds(struct nfs4_pnfs_ds *ds) { dprintk("--> %s\n", __func__); - ifdebug(FACILITY) - print_ds(ds); + print_ds(ds); if (ds->ds_clp) nfs_put_client(ds->ds_clp); -- 1.7.2.3 ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] SQUASHME: pnfs: filelayout: print_ds should use dprintk 2010-10-27 18:24 ` [PATCH 3/3] SQUASHME: pnfs: filelayout: print_ds should use dprintk Benny Halevy @ 2010-10-27 19:49 ` Fred Isaman 2010-10-27 20:06 ` Benny Halevy 0 siblings, 1 reply; 14+ messages in thread From: Fred Isaman @ 2010-10-27 19:49 UTC (permalink / raw) To: Benny Halevy; +Cc: linux-nfs The change to printk was in response to Trond's complaint about successive dprintks. Instead, the following would work: diff --git a/fs/nfs/nfs4filelayout.c b/fs/nfs/nfs4filelayout.c index 5f52e6f..2ce393c 100644 --- a/fs/nfs/nfs4filelayout.c +++ b/fs/nfs/nfs4filelayout.c @@ -585,7 +585,8 @@ filelayout_commit(struct nfs_write_data *data, int sync) } dprintk("%s: Initiating commit: %llu USE DS:\n", __func__, file_offset); - print_ds(ds); + ifdebug(FACILITY) + print_ds(ds); /* Send COMMIT to data server */ nfs_initiate_commit(dsdata, clnt, call_ops, sync); Fred On Wed, Oct 27, 2010 at 2:24 PM, Benny Halevy <bhalevy@panasas.com> wrote: > rather than printk to prevent printouts in non-debug mode > currently happening in filelayout_commit > > Signed-off-by: Benny Halevy <bhalevy@panasas.com> > --- > fs/nfs/nfs4filelayoutdev.c | 9 ++++----- > 1 files changed, 4 insertions(+), 5 deletions(-) > > diff --git a/fs/nfs/nfs4filelayoutdev.c b/fs/nfs/nfs4filelayoutdev.c > index 1f0ab62..de47112 100644 > --- a/fs/nfs/nfs4filelayoutdev.c > +++ b/fs/nfs/nfs4filelayoutdev.c > @@ -53,10 +53,10 @@ void > print_ds(struct nfs4_pnfs_ds *ds) > { > if (ds == NULL) { > - printk("%s NULL device\n", __func__); > + dprintk("%s NULL device\n", __func__); > return; > } > - printk(" ip_addr %x port %hu\n" > + dprintk(" ip_addr %x port %hu\n" > " ref count %d\n" > " client %p\n" > " cl_exchange_flags %x\n", > @@ -71,7 +71,7 @@ print_ds_list(struct nfs4_file_layout_dsaddr *dsaddr) > int i; > > ifdebug(FACILITY) { > - printk("%s dsaddr->ds_num %d\n", __func__, > + dprintk("%s dsaddr->ds_num %d\n", __func__, > dsaddr->ds_num); > for (i = 0; i < dsaddr->ds_num; i++) > print_ds(dsaddr->ds_list[i]); > @@ -211,8 +211,7 @@ static void > destroy_ds(struct nfs4_pnfs_ds *ds) > { > dprintk("--> %s\n", __func__); > - ifdebug(FACILITY) > - print_ds(ds); > + print_ds(ds); > > if (ds->ds_clp) > nfs_put_client(ds->ds_clp); > -- > 1.7.2.3 > > -- > 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 > ^ permalink raw reply related [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] SQUASHME: pnfs: filelayout: print_ds should use dprintk 2010-10-27 19:49 ` Fred Isaman @ 2010-10-27 20:06 ` Benny Halevy 2010-10-27 20:17 ` Fred Isaman 0 siblings, 1 reply; 14+ messages in thread From: Benny Halevy @ 2010-10-27 20:06 UTC (permalink / raw) To: Fred Isaman; +Cc: linux-nfs On 2010-10-27 21:49, Fred Isaman wrote: > The change to printk was in response to Trond's complaint about > successive dprintks. > > Instead, the following would work: > > > diff --git a/fs/nfs/nfs4filelayout.c b/fs/nfs/nfs4filelayout.c > index 5f52e6f..2ce393c 100644 > --- a/fs/nfs/nfs4filelayout.c > +++ b/fs/nfs/nfs4filelayout.c > @@ -585,7 +585,8 @@ filelayout_commit(struct nfs_write_data *data, int sync) > } If we're going this way, the ifdebug could cover the following printout as well... Benny > dprintk("%s: Initiating commit: %llu USE DS:\n", > __func__, file_offset); > - print_ds(ds); > + ifdebug(FACILITY) > + print_ds(ds); > > /* Send COMMIT to data server */ > nfs_initiate_commit(dsdata, clnt, call_ops, sync); > > > Fred > > On Wed, Oct 27, 2010 at 2:24 PM, Benny Halevy <bhalevy@panasas.com> wrote: >> rather than printk to prevent printouts in non-debug mode >> currently happening in filelayout_commit >> >> Signed-off-by: Benny Halevy <bhalevy@panasas.com> >> --- >> fs/nfs/nfs4filelayoutdev.c | 9 ++++----- >> 1 files changed, 4 insertions(+), 5 deletions(-) >> >> diff --git a/fs/nfs/nfs4filelayoutdev.c b/fs/nfs/nfs4filelayoutdev.c >> index 1f0ab62..de47112 100644 >> --- a/fs/nfs/nfs4filelayoutdev.c >> +++ b/fs/nfs/nfs4filelayoutdev.c >> @@ -53,10 +53,10 @@ void >> print_ds(struct nfs4_pnfs_ds *ds) >> { >> if (ds == NULL) { >> - printk("%s NULL device\n", __func__); >> + dprintk("%s NULL device\n", __func__); >> return; >> } >> - printk(" ip_addr %x port %hu\n" >> + dprintk(" ip_addr %x port %hu\n" >> " ref count %d\n" >> " client %p\n" >> " cl_exchange_flags %x\n", >> @@ -71,7 +71,7 @@ print_ds_list(struct nfs4_file_layout_dsaddr *dsaddr) >> int i; >> >> ifdebug(FACILITY) { >> - printk("%s dsaddr->ds_num %d\n", __func__, >> + dprintk("%s dsaddr->ds_num %d\n", __func__, >> dsaddr->ds_num); >> for (i = 0; i < dsaddr->ds_num; i++) >> print_ds(dsaddr->ds_list[i]); >> @@ -211,8 +211,7 @@ static void >> destroy_ds(struct nfs4_pnfs_ds *ds) >> { >> dprintk("--> %s\n", __func__); >> - ifdebug(FACILITY) >> - print_ds(ds); >> + print_ds(ds); >> >> if (ds->ds_clp) >> nfs_put_client(ds->ds_clp); >> -- >> 1.7.2.3 >> >> -- >> 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 >> ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] SQUASHME: pnfs: filelayout: print_ds should use dprintk 2010-10-27 20:06 ` Benny Halevy @ 2010-10-27 20:17 ` Fred Isaman 2010-10-27 21:00 ` Benny Halevy 0 siblings, 1 reply; 14+ messages in thread From: Fred Isaman @ 2010-10-27 20:17 UTC (permalink / raw) To: Benny Halevy; +Cc: linux-nfs On Wed, Oct 27, 2010 at 4:06 PM, Benny Halevy <bhalevy@panasas.com> wrote: > On 2010-10-27 21:49, Fred Isaman wrote: >> The change to printk was in response to Trond's complaint about >> successive dprintks. >> >> Instead, the following would work: >> >> >> diff --git a/fs/nfs/nfs4filelayout.c b/fs/nfs/nfs4filelayout.c >> index 5f52e6f..2ce393c 100644 >> --- a/fs/nfs/nfs4filelayout.c >> +++ b/fs/nfs/nfs4filelayout.c >> @@ -585,7 +585,8 @@ filelayout_commit(struct nfs_write_data *data, int sync) >> } > > If we're going this way, the ifdebug could cover the following > printout as well... > Did you mean preceding printout? By the way - the complaint about successive dprintks was regarding print_ds_list repeatedly calling print_ds, which at the time used dprintk. Fred > Benny > >> dprintk("%s: Initiating commit: %llu USE DS:\n", >> __func__, file_offset); >> - print_ds(ds); >> + ifdebug(FACILITY) >> + print_ds(ds); >> >> /* Send COMMIT to data server */ >> nfs_initiate_commit(dsdata, clnt, call_ops, sync); >> >> >> Fred >> >> On Wed, Oct 27, 2010 at 2:24 PM, Benny Halevy <bhalevy@panasas.com> wrote: >>> rather than printk to prevent printouts in non-debug mode >>> currently happening in filelayout_commit >>> >>> Signed-off-by: Benny Halevy <bhalevy@panasas.com> >>> --- >>> fs/nfs/nfs4filelayoutdev.c | 9 ++++----- >>> 1 files changed, 4 insertions(+), 5 deletions(-) >>> >>> diff --git a/fs/nfs/nfs4filelayoutdev.c b/fs/nfs/nfs4filelayoutdev.c >>> index 1f0ab62..de47112 100644 >>> --- a/fs/nfs/nfs4filelayoutdev.c >>> +++ b/fs/nfs/nfs4filelayoutdev.c >>> @@ -53,10 +53,10 @@ void >>> print_ds(struct nfs4_pnfs_ds *ds) >>> { >>> if (ds == NULL) { >>> - printk("%s NULL device\n", __func__); >>> + dprintk("%s NULL device\n", __func__); >>> return; >>> } >>> - printk(" ip_addr %x port %hu\n" >>> + dprintk(" ip_addr %x port %hu\n" >>> " ref count %d\n" >>> " client %p\n" >>> " cl_exchange_flags %x\n", >>> @@ -71,7 +71,7 @@ print_ds_list(struct nfs4_file_layout_dsaddr *dsaddr) >>> int i; >>> >>> ifdebug(FACILITY) { >>> - printk("%s dsaddr->ds_num %d\n", __func__, >>> + dprintk("%s dsaddr->ds_num %d\n", __func__, >>> dsaddr->ds_num); >>> for (i = 0; i < dsaddr->ds_num; i++) >>> print_ds(dsaddr->ds_list[i]); >>> @@ -211,8 +211,7 @@ static void >>> destroy_ds(struct nfs4_pnfs_ds *ds) >>> { >>> dprintk("--> %s\n", __func__); >>> - ifdebug(FACILITY) >>> - print_ds(ds); >>> + print_ds(ds); >>> >>> if (ds->ds_clp) >>> nfs_put_client(ds->ds_clp); >>> -- >>> 1.7.2.3 >>> >>> -- >>> 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 > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] SQUASHME: pnfs: filelayout: print_ds should use dprintk 2010-10-27 20:17 ` Fred Isaman @ 2010-10-27 21:00 ` Benny Halevy 2010-10-27 21:23 ` Trond Myklebust 0 siblings, 1 reply; 14+ messages in thread From: Benny Halevy @ 2010-10-27 21:00 UTC (permalink / raw) To: Fred Isaman; +Cc: linux-nfs On 2010-10-27 22:17, Fred Isaman wrote: > On Wed, Oct 27, 2010 at 4:06 PM, Benny Halevy <bhalevy@panasas.com> wrote: >> On 2010-10-27 21:49, Fred Isaman wrote: >>> The change to printk was in response to Trond's complaint about >>> successive dprintks. >>> >>> Instead, the following would work: >>> >>> >>> diff --git a/fs/nfs/nfs4filelayout.c b/fs/nfs/nfs4filelayout.c >>> index 5f52e6f..2ce393c 100644 >>> --- a/fs/nfs/nfs4filelayout.c >>> +++ b/fs/nfs/nfs4filelayout.c >>> @@ -585,7 +585,8 @@ filelayout_commit(struct nfs_write_data *data, int sync) >>> } >> >> If we're going this way, the ifdebug could cover the following >> printout as well... >> > > Did you mean preceding printout? By the way - the complaint about Yeah, preceding the call to print_ds (following my comment :) > successive dprintks was regarding > print_ds_list repeatedly calling print_ds, which at the time used dprintk. Why do we care to optimize the debug case so much? print_ds_list is already calling print_ds inside ifdebug(FACILITY) so the common, non-debug case is optimized correctly. I.e. we don't repeatedly check the debug flag normally. Benny > > Fred > >> Benny >> >>> dprintk("%s: Initiating commit: %llu USE DS:\n", >>> __func__, file_offset); >>> - print_ds(ds); >>> + ifdebug(FACILITY) >>> + print_ds(ds); >>> >>> /* Send COMMIT to data server */ >>> nfs_initiate_commit(dsdata, clnt, call_ops, sync); >>> >>> >>> Fred >>> >>> On Wed, Oct 27, 2010 at 2:24 PM, Benny Halevy <bhalevy@panasas.com> wrote: >>>> rather than printk to prevent printouts in non-debug mode >>>> currently happening in filelayout_commit >>>> >>>> Signed-off-by: Benny Halevy <bhalevy@panasas.com> >>>> --- >>>> fs/nfs/nfs4filelayoutdev.c | 9 ++++----- >>>> 1 files changed, 4 insertions(+), 5 deletions(-) >>>> >>>> diff --git a/fs/nfs/nfs4filelayoutdev.c b/fs/nfs/nfs4filelayoutdev.c >>>> index 1f0ab62..de47112 100644 >>>> --- a/fs/nfs/nfs4filelayoutdev.c >>>> +++ b/fs/nfs/nfs4filelayoutdev.c >>>> @@ -53,10 +53,10 @@ void >>>> print_ds(struct nfs4_pnfs_ds *ds) >>>> { >>>> if (ds == NULL) { >>>> - printk("%s NULL device\n", __func__); >>>> + dprintk("%s NULL device\n", __func__); >>>> return; >>>> } >>>> - printk(" ip_addr %x port %hu\n" >>>> + dprintk(" ip_addr %x port %hu\n" >>>> " ref count %d\n" >>>> " client %p\n" >>>> " cl_exchange_flags %x\n", >>>> @@ -71,7 +71,7 @@ print_ds_list(struct nfs4_file_layout_dsaddr *dsaddr) >>>> int i; >>>> >>>> ifdebug(FACILITY) { >>>> - printk("%s dsaddr->ds_num %d\n", __func__, >>>> + dprintk("%s dsaddr->ds_num %d\n", __func__, >>>> dsaddr->ds_num); >>>> for (i = 0; i < dsaddr->ds_num; i++) >>>> print_ds(dsaddr->ds_list[i]); >>>> @@ -211,8 +211,7 @@ static void >>>> destroy_ds(struct nfs4_pnfs_ds *ds) >>>> { >>>> dprintk("--> %s\n", __func__); >>>> - ifdebug(FACILITY) >>>> - print_ds(ds); >>>> + print_ds(ds); >>>> >>>> if (ds->ds_clp) >>>> nfs_put_client(ds->ds_clp); >>>> -- >>>> 1.7.2.3 >>>> >>>> -- >>>> 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 >> ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] SQUASHME: pnfs: filelayout: print_ds should use dprintk 2010-10-27 21:00 ` Benny Halevy @ 2010-10-27 21:23 ` Trond Myklebust 2010-10-27 21:29 ` Benny Halevy 0 siblings, 1 reply; 14+ messages in thread From: Trond Myklebust @ 2010-10-27 21:23 UTC (permalink / raw) To: Benny Halevy; +Cc: Fred Isaman, linux-nfs On Wed, 2010-10-27 at 23:00 +0200, Benny Halevy wrote: > On 2010-10-27 22:17, Fred Isaman wrote: > > On Wed, Oct 27, 2010 at 4:06 PM, Benny Halevy <bhalevy@panasas.com> wrote: > >> On 2010-10-27 21:49, Fred Isaman wrote: > >>> The change to printk was in response to Trond's complaint about > >>> successive dprintks. > >>> > >>> Instead, the following would work: > >>> > >>> > >>> diff --git a/fs/nfs/nfs4filelayout.c b/fs/nfs/nfs4filelayout.c > >>> index 5f52e6f..2ce393c 100644 > >>> --- a/fs/nfs/nfs4filelayout.c > >>> +++ b/fs/nfs/nfs4filelayout.c > >>> @@ -585,7 +585,8 @@ filelayout_commit(struct nfs_write_data *data, int sync) > >>> } > >> > >> If we're going this way, the ifdebug could cover the following > >> printout as well... > >> > > > > Did you mean preceding printout? By the way - the complaint about > > Yeah, preceding the call to print_ds (following my comment :) > > > successive dprintks was regarding > > print_ds_list repeatedly calling print_ds, which at the time used dprintk. > > Why do we care to optimize the debug case so much? > print_ds_list is already calling print_ds inside ifdebug(FACILITY) > so the common, non-debug case is optimized correctly. I.e. we don't > repeatedly check the debug flag normally. It's not about optimizing the debug case. It's about avoiding having to check ifdebug(FACILITY) all the time when we're _not_ debugging. Trond ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] SQUASHME: pnfs: filelayout: print_ds should use dprintk 2010-10-27 21:23 ` Trond Myklebust @ 2010-10-27 21:29 ` Benny Halevy 2010-10-27 21:32 ` Trond Myklebust 0 siblings, 1 reply; 14+ messages in thread From: Benny Halevy @ 2010-10-27 21:29 UTC (permalink / raw) To: Trond Myklebust; +Cc: Fred Isaman, linux-nfs On 2010-10-27 23:23, Trond Myklebust wrote: > On Wed, 2010-10-27 at 23:00 +0200, Benny Halevy wrote: >> On 2010-10-27 22:17, Fred Isaman wrote: >>> On Wed, Oct 27, 2010 at 4:06 PM, Benny Halevy <bhalevy@panasas.com> wrote: >>>> On 2010-10-27 21:49, Fred Isaman wrote: >>>>> The change to printk was in response to Trond's complaint about >>>>> successive dprintks. >>>>> >>>>> Instead, the following would work: >>>>> >>>>> >>>>> diff --git a/fs/nfs/nfs4filelayout.c b/fs/nfs/nfs4filelayout.c >>>>> index 5f52e6f..2ce393c 100644 >>>>> --- a/fs/nfs/nfs4filelayout.c >>>>> +++ b/fs/nfs/nfs4filelayout.c >>>>> @@ -585,7 +585,8 @@ filelayout_commit(struct nfs_write_data *data, int sync) >>>>> } >>>> >>>> If we're going this way, the ifdebug could cover the following >>>> printout as well... >>>> >>> >>> Did you mean preceding printout? By the way - the complaint about >> >> Yeah, preceding the call to print_ds (following my comment :) >> >>> successive dprintks was regarding >>> print_ds_list repeatedly calling print_ds, which at the time used dprintk. >> >> Why do we care to optimize the debug case so much? >> print_ds_list is already calling print_ds inside ifdebug(FACILITY) >> so the common, non-debug case is optimized correctly. I.e. we don't >> repeatedly check the debug flag normally. > > It's not about optimizing the debug case. It's about avoiding having to > check ifdebug(FACILITY) all the time when we're _not_ debugging. Right, and so we do, as the whole loop in print_ds_list is enclosed in ifdebug(FACILITY). Benny > > Trond > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] SQUASHME: pnfs: filelayout: print_ds should use dprintk 2010-10-27 21:29 ` Benny Halevy @ 2010-10-27 21:32 ` Trond Myklebust 2010-10-27 21:42 ` Benny Halevy 0 siblings, 1 reply; 14+ messages in thread From: Trond Myklebust @ 2010-10-27 21:32 UTC (permalink / raw) To: Benny Halevy; +Cc: Fred Isaman, linux-nfs On Wed, 2010-10-27 at 23:29 +0200, Benny Halevy wrote: > On 2010-10-27 23:23, Trond Myklebust wrote: > > On Wed, 2010-10-27 at 23:00 +0200, Benny Halevy wrote: > >> On 2010-10-27 22:17, Fred Isaman wrote: > >>> On Wed, Oct 27, 2010 at 4:06 PM, Benny Halevy <bhalevy@panasas.com> wrote: > >>>> On 2010-10-27 21:49, Fred Isaman wrote: > >>>>> The change to printk was in response to Trond's complaint about > >>>>> successive dprintks. > >>>>> > >>>>> Instead, the following would work: > >>>>> > >>>>> > >>>>> diff --git a/fs/nfs/nfs4filelayout.c b/fs/nfs/nfs4filelayout.c > >>>>> index 5f52e6f..2ce393c 100644 > >>>>> --- a/fs/nfs/nfs4filelayout.c > >>>>> +++ b/fs/nfs/nfs4filelayout.c > >>>>> @@ -585,7 +585,8 @@ filelayout_commit(struct nfs_write_data *data, int sync) > >>>>> } > >>>> > >>>> If we're going this way, the ifdebug could cover the following > >>>> printout as well... > >>>> > >>> > >>> Did you mean preceding printout? By the way - the complaint about > >> > >> Yeah, preceding the call to print_ds (following my comment :) > >> > >>> successive dprintks was regarding > >>> print_ds_list repeatedly calling print_ds, which at the time used dprintk. > >> > >> Why do we care to optimize the debug case so much? > >> print_ds_list is already calling print_ds inside ifdebug(FACILITY) > >> so the common, non-debug case is optimized correctly. I.e. we don't > >> repeatedly check the debug flag normally. > > > > It's not about optimizing the debug case. It's about avoiding having to > > check ifdebug(FACILITY) all the time when we're _not_ debugging. > > Right, and so we do, as the whole loop in print_ds_list is enclosed > in ifdebug(FACILITY). In that case, I agree: the whole thing can be converted to use ordinary printks... Trond ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] SQUASHME: pnfs: filelayout: print_ds should use dprintk 2010-10-27 21:32 ` Trond Myklebust @ 2010-10-27 21:42 ` Benny Halevy 2010-10-28 13:11 ` Benny Halevy 0 siblings, 1 reply; 14+ messages in thread From: Benny Halevy @ 2010-10-27 21:42 UTC (permalink / raw) To: Trond Myklebust; +Cc: Fred Isaman, linux-nfs On 2010-10-27 23:32, Trond Myklebust wrote: > On Wed, 2010-10-27 at 23:29 +0200, Benny Halevy wrote: >> On 2010-10-27 23:23, Trond Myklebust wrote: >>> On Wed, 2010-10-27 at 23:00 +0200, Benny Halevy wrote: >>>> On 2010-10-27 22:17, Fred Isaman wrote: >>>>> On Wed, Oct 27, 2010 at 4:06 PM, Benny Halevy <bhalevy@panasas.com> wrote: >>>>>> On 2010-10-27 21:49, Fred Isaman wrote: >>>>>>> The change to printk was in response to Trond's complaint about >>>>>>> successive dprintks. >>>>>>> >>>>>>> Instead, the following would work: >>>>>>> >>>>>>> >>>>>>> diff --git a/fs/nfs/nfs4filelayout.c b/fs/nfs/nfs4filelayout.c >>>>>>> index 5f52e6f..2ce393c 100644 >>>>>>> --- a/fs/nfs/nfs4filelayout.c >>>>>>> +++ b/fs/nfs/nfs4filelayout.c >>>>>>> @@ -585,7 +585,8 @@ filelayout_commit(struct nfs_write_data *data, int sync) >>>>>>> } >>>>>> >>>>>> If we're going this way, the ifdebug could cover the following >>>>>> printout as well... >>>>>> >>>>> >>>>> Did you mean preceding printout? By the way - the complaint about >>>> >>>> Yeah, preceding the call to print_ds (following my comment :) >>>> >>>>> successive dprintks was regarding >>>>> print_ds_list repeatedly calling print_ds, which at the time used dprintk. >>>> >>>> Why do we care to optimize the debug case so much? >>>> print_ds_list is already calling print_ds inside ifdebug(FACILITY) >>>> so the common, non-debug case is optimized correctly. I.e. we don't >>>> repeatedly check the debug flag normally. >>> >>> It's not about optimizing the debug case. It's about avoiding having to >>> check ifdebug(FACILITY) all the time when we're _not_ debugging. >> >> Right, and so we do, as the whole loop in print_ds_list is enclosed >> in ifdebug(FACILITY). > > In that case, I agree: the whole thing can be converted to use ordinary > printks... The point is it has a singular caller outside of this loop for which the caller needs to check ifdebug(FACILITY) before calling print_ds. Not a big deal, but I think it'd be cleaner if print_ds will do a dprintk to simplify its singular usage, while the cost of that will be extra tests when called, possibly many times, in debug mode from print_ds_list. Benny > > Trond > ^ permalink raw reply [flat|nested] 14+ messages in thread
* Re: [PATCH 3/3] SQUASHME: pnfs: filelayout: print_ds should use dprintk 2010-10-27 21:42 ` Benny Halevy @ 2010-10-28 13:11 ` Benny Halevy 0 siblings, 0 replies; 14+ messages in thread From: Benny Halevy @ 2010-10-28 13:11 UTC (permalink / raw) To: Trond Myklebust, Fred Isaman; +Cc: linux-nfs On 2010-10-27 23:42, Benny Halevy wrote: > On 2010-10-27 23:32, Trond Myklebust wrote: >> On Wed, 2010-10-27 at 23:29 +0200, Benny Halevy wrote: >>> On 2010-10-27 23:23, Trond Myklebust wrote: >>>> On Wed, 2010-10-27 at 23:00 +0200, Benny Halevy wrote: >>>>> On 2010-10-27 22:17, Fred Isaman wrote: >>>>>> On Wed, Oct 27, 2010 at 4:06 PM, Benny Halevy <bhalevy@panasas.com> wrote: >>>>>>> On 2010-10-27 21:49, Fred Isaman wrote: >>>>>>>> The change to printk was in response to Trond's complaint about >>>>>>>> successive dprintks. >>>>>>>> >>>>>>>> Instead, the following would work: >>>>>>>> >>>>>>>> >>>>>>>> diff --git a/fs/nfs/nfs4filelayout.c b/fs/nfs/nfs4filelayout.c >>>>>>>> index 5f52e6f..2ce393c 100644 >>>>>>>> --- a/fs/nfs/nfs4filelayout.c >>>>>>>> +++ b/fs/nfs/nfs4filelayout.c >>>>>>>> @@ -585,7 +585,8 @@ filelayout_commit(struct nfs_write_data *data, int sync) >>>>>>>> } >>>>>>> >>>>>>> If we're going this way, the ifdebug could cover the following >>>>>>> printout as well... >>>>>>> >>>>>> >>>>>> Did you mean preceding printout? By the way - the complaint about >>>>> >>>>> Yeah, preceding the call to print_ds (following my comment :) >>>>> >>>>>> successive dprintks was regarding >>>>>> print_ds_list repeatedly calling print_ds, which at the time used dprintk. >>>>> >>>>> Why do we care to optimize the debug case so much? >>>>> print_ds_list is already calling print_ds inside ifdebug(FACILITY) >>>>> so the common, non-debug case is optimized correctly. I.e. we don't >>>>> repeatedly check the debug flag normally. >>>> >>>> It's not about optimizing the debug case. It's about avoiding having to >>>> check ifdebug(FACILITY) all the time when we're _not_ debugging. >>> >>> Right, and so we do, as the whole loop in print_ds_list is enclosed >>> in ifdebug(FACILITY). >> >> In that case, I agree: the whole thing can be converted to use ordinary >> printks... > > The point is it has a singular caller outside of this loop for which > the caller needs to check ifdebug(FACILITY) before calling print_ds. > Not a big deal, but I think it'd be cleaner if print_ds will do a dprintk > to simplify its singular usage, while the cost of that will be extra tests > when called, possibly many times, in debug mode from print_ds_list. Nevermind, I just merged Fred's patch instead. No need to spend that much energy on cosmetics in this case :) Benny > > Benny > >> >> Trond >> > -- > 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 ^ permalink raw reply [flat|nested] 14+ messages in thread
end of thread, other threads:[~2010-10-28 13:11 UTC | newest] Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2010-10-27 18:21 [PATCH 1/4] pnfs fixes Benny Halevy 2010-10-27 18:22 ` [PATCH 1/1] SQUASHME: pnfsd-files: update layout stateid properly Benny Halevy 2010-10-27 18:23 ` [PATCH 1/1] pnfs: do not change layout stateid when dropping layouts Benny Halevy 2010-10-27 18:24 ` [PATCH 2/3] pnfs: mark page with error in readpage_async_filler when crossing lsegs Benny Halevy 2010-10-27 18:24 ` [PATCH 3/3] SQUASHME: pnfs: filelayout: print_ds should use dprintk Benny Halevy 2010-10-27 19:49 ` Fred Isaman 2010-10-27 20:06 ` Benny Halevy 2010-10-27 20:17 ` Fred Isaman 2010-10-27 21:00 ` Benny Halevy 2010-10-27 21:23 ` Trond Myklebust 2010-10-27 21:29 ` Benny Halevy 2010-10-27 21:32 ` Trond Myklebust 2010-10-27 21:42 ` Benny Halevy 2010-10-28 13:11 ` Benny Halevy
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.