From: Eric Sandeen <hidden> Date: 2014-08-22 00:55:21
Move the resblks test out of the xfs_dir_canenter,
and into the caller.
This makes a little more sense on the face of it;
xfs_dir_canenter immediately returns if resblks !=0;
and given some of the comments preceding the calls:
* Check for ability to enter directory entry, if no space reserved.
even more so.
It also facilitates the next patch.
Signed-off-by: Eric Sandeen <redacted>
---
@@ -535,22 +535,17 @@ out_free:/**Seeifthisentrycanbeaddedtothedirectorywithoutallocatingspace.-*Firstchecksthatthecallercouldn'treserveenoughspace(resblks=0).*/intxfs_dir_canenter(xfs_trans_t*tp,xfs_inode_t*dp,-structxfs_name*name,/* name of entry to add */-uintresblks)+structxfs_name*name)/* name of entry to add */{structxfs_da_args*args;intrval;intv;/* type-checking value */-if(resblks)-return0;-ASSERT(S_ISDIR(dp->i_d.di_mode));args=kmem_zalloc(sizeof(*args),KM_SLEEP|KM_NOFS);
From: Eric Sandeen <hidden> Date: 2014-08-22 00:58:39
xfs_dir_canenter and xfs_dir_createname are
almost identical.
Fold the former into the latter, with a helpful
wrapper for the former. If createname is called without
an inode number, it now only checks for space, and does
not actually add the entry.
Signed-off-by: Eric Sandeen <redacted>
---
@@ -542,50 +547,7 @@ xfs_dir_canenter(xfs_inode_t*dp,structxfs_name*name)/* name of entry to add */{-structxfs_da_args*args;-intrval;-intv;/* type-checking value */--ASSERT(S_ISDIR(dp->i_d.di_mode));--args=kmem_zalloc(sizeof(*args),KM_SLEEP|KM_NOFS);-if(!args)-return-ENOMEM;--args->geo=dp->i_mount->m_dir_geo;-args->name=name->name;-args->namelen=name->len;-args->filetype=name->type;-args->hashval=dp->i_mount->m_dirnameops->hashname(name);-args->dp=dp;-args->whichfork=XFS_DATA_FORK;-args->trans=tp;-args->op_flags=XFS_DA_OP_JUSTCHECK|XFS_DA_OP_ADDNAME|-XFS_DA_OP_OKNOENT;--if(dp->i_d.di_format==XFS_DINODE_FMT_LOCAL){-rval=xfs_dir2_sf_addname(args);-gotoout_free;-}--rval=xfs_dir2_isblock(args,&v);-if(rval)-gotoout_free;-if(v){-rval=xfs_dir2_block_addname(args);-gotoout_free;-}--rval=xfs_dir2_isleaf(args,&v);-if(rval)-gotoout_free;-if(v)-rval=xfs_dir2_leaf_addname(args);-else-rval=xfs_dir2_node_addname(args);-out_free:-kmem_free(args);-returnrval;+returnxfs_dir_createname(tp,dp,name,0,NULL,NULL,0);}/*
From: Eric Sandeen <hidden> Date: 2014-08-22 01:00:41
xfs_rtmodify_summary and xfs_rtget_summary are
almost identical; fold them into
xfs_rtmodify_summary_int(), with wrappers for
each of the original calls.
The _int function modifies if a delta is passed,
and returns a summary pointer if *sum is passed.
Signed-off-by: Eric Sandeen <redacted>
---
@@ -424,20 +424,24 @@ xfs_rtfind_forw(}/*-*Readandmodifythesummaryinformationforagivenextentsize,+*Readand/ormodifythesummaryinformationforagivenextentsize,*bitmapblockcombination.*Keepstrackofacurrentsummaryblock,sowedon'tkeepreading*itfromthebuffercache.+*+*Summaryinformationisreturnedin*sumifspecified.+*Ifnodeltaisspecified,returnssummaryonly.*/int-xfs_rtmodify_summary(-xfs_mount_t*mp,/* file system mount point */+xfs_rtmodify_summary_int(+xfs_mount_t*mp,/* file system mount structure */xfs_trans_t*tp,/* transaction pointer */intlog,/* log2 of extent size */xfs_rtblock_tbbno,/* bitmap block number */intdelta,/* change to make to summary info */xfs_buf_t**rbpp,/* in/out: summary block buffer */-xfs_fsblock_t*rsb)/* in/out: summary block number */+xfs_fsblock_t*rsb,/* in/out: summary block number */+xfs_suminfo_t*sum)/* out: summary info for this block */{xfs_buf_t*bp;/* buffer for the summary block */interror;/* error value */
@@ -480,15 +484,40 @@ xfs_rtmodify_summary(}}/*-*Pointtothesummaryinformation,modifyandlogit.+*Pointtothesummaryinformation,modify/logit,and/orcopyitout.*/sp=XFS_SUMPTR(mp,bp,so);-*sp+=delta;-xfs_trans_log_buf(tp,bp,(uint)((char*)sp-(char*)bp->b_addr),-(uint)((char*)sp-(char*)bp->b_addr+sizeof(*sp)-1));+if(delta){+uintfirst=(uint)((char*)sp-(char*)bp->b_addr);++*sp+=delta;+xfs_trans_log_buf(tp,bp,first,first+sizeof(*sp)-1);+}+if(sum){+/*+*Dropthebufferifwe'renotaskedtorememberit.+*/+if(!rbpp)+xfs_trans_brelse(tp,bp);+*sum=*sp;+}return0;}+int+xfs_rtmodify_summary(+xfs_mount_t*mp,/* file system mount structure */+xfs_trans_t*tp,/* transaction pointer */+intlog,/* log2 of extent size */+xfs_rtblock_tbbno,/* bitmap block number */+intdelta,/* change to make to summary info */+xfs_buf_t**rbpp,/* in/out: summary block buffer */+xfs_fsblock_t*rsb)/* in/out: summary block number */+{+returnxfs_rtmodify_summary_int(mp,tp,log,bbno,+delta,rbpp,rsb,NULL);+}+/**Setthegivenrangeofbitmapbitstothegivenvalue.*DowhateverI/Oandloggingisrequired.
From: Eric Sandeen <hidden> Date: 2014-08-22 01:03:18
rbpp is always passed into xfs_rtmodify_summary
and xfs_rtget_summary, so there is no need to
test for it in xfs_rtmodify_summary_int.
Signed-off-by: Eric Sandeen <redacted>
---
From: Brian Foster <hidden> Date: 2014-08-22 13:19:38
On Thu, Aug 21, 2014 at 07:55:21PM -0500, Eric Sandeen wrote:
Move the resblks test out of the xfs_dir_canenter,
and into the caller.
This makes a little more sense on the face of it;
xfs_dir_canenter immediately returns if resblks !=0;
and given some of the comments preceding the calls:
* Check for ability to enter directory entry, if no space reserved.
even more so.
It also facilitates the next patch.
Signed-off-by: Eric Sandeen <redacted>
---
@@ -535,22 +535,17 @@ out_free:/**Seeifthisentrycanbeaddedtothedirectorywithoutallocatingspace.-*Firstchecksthatthecallercouldn'treserveenoughspace(resblks=0).*/intxfs_dir_canenter(xfs_trans_t*tp,xfs_inode_t*dp,-structxfs_name*name,/* name of entry to add */-uintresblks)+structxfs_name*name)/* name of entry to add */{structxfs_da_args*args;intrval;intv;/* type-checking value */-if(resblks)-return0;-ASSERT(S_ISDIR(dp->i_d.di_mode));args=kmem_zalloc(sizeof(*args),KM_SLEEP|KM_NOFS);
From: Brian Foster <hidden> Date: 2014-08-22 13:19:44
On Thu, Aug 21, 2014 at 07:58:43PM -0500, Eric Sandeen wrote:
xfs_dir_canenter and xfs_dir_createname are
almost identical.
Fold the former into the latter, with a helpful
wrapper for the former. If createname is called without
an inode number, it now only checks for space, and does
not actually add the entry.
Signed-off-by: Eric Sandeen <redacted>
---
@@ -542,50 +547,7 @@ xfs_dir_canenter(xfs_inode_t*dp,structxfs_name*name)/* name of entry to add */{-structxfs_da_args*args;-intrval;-intv;/* type-checking value */--ASSERT(S_ISDIR(dp->i_d.di_mode));--args=kmem_zalloc(sizeof(*args),KM_SLEEP|KM_NOFS);-if(!args)-return-ENOMEM;--args->geo=dp->i_mount->m_dir_geo;-args->name=name->name;-args->namelen=name->len;-args->filetype=name->type;-args->hashval=dp->i_mount->m_dirnameops->hashname(name);-args->dp=dp;-args->whichfork=XFS_DATA_FORK;-args->trans=tp;-args->op_flags=XFS_DA_OP_JUSTCHECK|XFS_DA_OP_ADDNAME|-XFS_DA_OP_OKNOENT;--if(dp->i_d.di_format==XFS_DINODE_FMT_LOCAL){-rval=xfs_dir2_sf_addname(args);-gotoout_free;-}--rval=xfs_dir2_isblock(args,&v);-if(rval)-gotoout_free;-if(v){-rval=xfs_dir2_block_addname(args);-gotoout_free;-}--rval=xfs_dir2_isleaf(args,&v);-if(rval)-gotoout_free;-if(v)-rval=xfs_dir2_leaf_addname(args);-else-rval=xfs_dir2_node_addname(args);-out_free:-kmem_free(args);-returnrval;+returnxfs_dir_createname(tp,dp,name,0,NULL,NULL,0);}/*
From: Brian Foster <hidden> Date: 2014-08-22 13:19:58
On Thu, Aug 21, 2014 at 08:00:45PM -0500, Eric Sandeen wrote:
quoted hunk
xfs_rtmodify_summary and xfs_rtget_summary are
almost identical; fold them into
xfs_rtmodify_summary_int(), with wrappers for
each of the original calls.
The _int function modifies if a delta is passed,
and returns a summary pointer if *sum is passed.
Signed-off-by: Eric Sandeen <redacted>
---
@@ -424,20 +424,24 @@ xfs_rtfind_forw(}/*-*Readandmodifythesummaryinformationforagivenextentsize,+*Readand/ormodifythesummaryinformationforagivenextentsize,*bitmapblockcombination.*Keepstrackofacurrentsummaryblock,sowedon'tkeepreading*itfromthebuffercache.+*+*Summaryinformationisreturnedin*sumifspecified.+*Ifnodeltaisspecified,returnssummaryonly.*/int-xfs_rtmodify_summary(-xfs_mount_t*mp,/* file system mount point */+xfs_rtmodify_summary_int(+xfs_mount_t*mp,/* file system mount structure */xfs_trans_t*tp,/* transaction pointer */intlog,/* log2 of extent size */xfs_rtblock_tbbno,/* bitmap block number */intdelta,/* change to make to summary info */xfs_buf_t**rbpp,/* in/out: summary block buffer */-xfs_fsblock_t*rsb)/* in/out: summary block number */+xfs_fsblock_t*rsb,/* in/out: summary block number */+xfs_suminfo_t*sum)/* out: summary info for this block */{xfs_buf_t*bp;/* buffer for the summary block */interror;/* error value */
This introduces some potentially weird circumstances (e.g., acquire,
log, release of a buffer), but I think it's resolved by the next patch.
Reviewed-by: Brian Foster <redacted>
quoted hunk
+ *sum = *sp;+ } return 0; }+int+xfs_rtmodify_summary(+ xfs_mount_t *mp, /* file system mount structure */+ xfs_trans_t *tp, /* transaction pointer */+ int log, /* log2 of extent size */+ xfs_rtblock_t bbno, /* bitmap block number */+ int delta, /* change to make to summary info */+ xfs_buf_t **rbpp, /* in/out: summary block buffer */+ xfs_fsblock_t *rsb) /* in/out: summary block number */+{+ return xfs_rtmodify_summary_int(mp, tp, log, bbno,+ delta, rbpp, rsb, NULL);+}+ /* * Set the given range of bitmap bits to the given value. * Do whatever I/O and logging is required.
From: Eric Sandeen <hidden> Date: 2014-08-22 15:01:38
On 8/22/14, 8:19 AM, Brian Foster wrote:
On Thu, Aug 21, 2014 at 08:00:45PM -0500, Eric Sandeen wrote:
...
quoted
@@ -480,15 +484,40 @@ xfs_rtmodify_summary( } } /*- * Point to the summary information, modify and log it.+ * Point to the summary information, modify/log it, and/or copy it out. */ sp = XFS_SUMPTR(mp, bp, so);- *sp += delta;- xfs_trans_log_buf(tp, bp, (uint)((char *)sp - (char *)bp->b_addr),- (uint)((char *)sp - (char *)bp->b_addr + sizeof(*sp) - 1));+ if (delta) {+ uint first = (uint)((char *)sp - (char *)bp->b_addr);++ *sp += delta;+ xfs_trans_log_buf(tp, bp, first, first + sizeof(*sp) - 1);+ }+ if (sum) {+ /*+ * Drop the buffer if we're not asked to remember it.+ */+ if (!rbpp)+ xfs_trans_brelse(tp, bp);
This introduces some potentially weird circumstances (e.g., acquire,
log, release of a buffer), but I think it's resolved by the next patch.
Reviewed-by: Brian Foster <redacted>
does it introduce it, or just highlight it? I thought it was weird too,
but I think it existed before; that's what prompted me to go looking at
callers and drop the rbpp checks, FWIW.
-Eric
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
From: Eric Sandeen <hidden> Date: 2014-08-22 15:23:40
On 8/22/14, 10:01 AM, Eric Sandeen wrote:
On 8/22/14, 8:19 AM, Brian Foster wrote:
quoted
On Thu, Aug 21, 2014 at 08:00:45PM -0500, Eric Sandeen wrote:
...
quoted
quoted
@@ -480,15 +484,40 @@ xfs_rtmodify_summary( } } /*- * Point to the summary information, modify and log it.+ * Point to the summary information, modify/log it, and/or copy it out. */ sp = XFS_SUMPTR(mp, bp, so);- *sp += delta;- xfs_trans_log_buf(tp, bp, (uint)((char *)sp - (char *)bp->b_addr),- (uint)((char *)sp - (char *)bp->b_addr + sizeof(*sp) - 1));+ if (delta) {+ uint first = (uint)((char *)sp - (char *)bp->b_addr);++ *sp += delta;+ xfs_trans_log_buf(tp, bp, first, first + sizeof(*sp) - 1);+ }+ if (sum) {+ /*+ * Drop the buffer if we're not asked to remember it.+ */+ if (!rbpp)+ xfs_trans_brelse(tp, bp);
This introduces some potentially weird circumstances (e.g., acquire,
log, release of a buffer), but I think it's resolved by the next patch.
Reviewed-by: Brian Foster <redacted>
does it introduce it, or just highlight it? I thought it was weird too,
but I think it existed before; that's what prompted me to go looking at
callers and drop the rbpp checks, FWIW.
Oh, right, if delta & sum,there's a problem if (!rpbb). And I did introduce
that as a new issue. But the code at that point never sees (!rbpp) so I think
it's safe. I could respin patches 3 & 4, to do them in the opposite order,
if desired.
-Eric
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
From: Christoph Hellwig <hch@infradead.org> Date: 2014-08-29 00:59:26
Looks good (although not useful on it's own, but there's more followups)
Reviewed-by: Christoph Hellwig <hch@lst.de>
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
From: Christoph Hellwig <hch@infradead.org> Date: 2014-08-29 01:00:59
On Thu, Aug 21, 2014 at 07:58:43PM -0500, Eric Sandeen wrote:
xfs_dir_canenter and xfs_dir_createname are
almost identical.
Fold the former into the latter, with a helpful
wrapper for the former. If createname is called without
an inode number, it now only checks for space, and does
not actually add the entry.
Signed-off-by: Eric Sandeen <redacted>
The code changes looks good to me, but..
quoted hunk
/*- Enter a name in a directory.+ * Enter a name in a directory.+ * If inum is 0, only test for available space. */ int xfs_dir_createname(
Given how confusing the xfs_dir_createname function name is now this
probably needs a more detailed description mentioning the checking as
first class behavior.
_______________________________________________
xfs mailing list
xfs@oss.sgi.com
http://oss.sgi.com/mailman/listinfo/xfs
From: Eric Sandeen <hidden> Date: 2014-08-29 02:14:09
xfs_dir_canenter and xfs_dir_createname are
almost identical.
Fold the former into the latter, with a helpful
wrapper for the former. If createname is called without
an inode number, it now only checks for space, and does
not actually add the entry.
Signed-off-by: Eric Sandeen <redacted>
---
V2: slightly more verbose comment
@@ -542,50 +547,7 @@ xfs_dir_canenter(xfs_inode_t*dp,structxfs_name*name)/* name of entry to add */{-structxfs_da_args*args;-intrval;-intv;/* type-checking value */--ASSERT(S_ISDIR(dp->i_d.di_mode));--args=kmem_zalloc(sizeof(*args),KM_SLEEP|KM_NOFS);-if(!args)-return-ENOMEM;--args->geo=dp->i_mount->m_dir_geo;-args->name=name->name;-args->namelen=name->len;-args->filetype=name->type;-args->hashval=dp->i_mount->m_dirnameops->hashname(name);-args->dp=dp;-args->whichfork=XFS_DATA_FORK;-args->trans=tp;-args->op_flags=XFS_DA_OP_JUSTCHECK|XFS_DA_OP_ADDNAME|-XFS_DA_OP_OKNOENT;--if(dp->i_d.di_format==XFS_DINODE_FMT_LOCAL){-rval=xfs_dir2_sf_addname(args);-gotoout_free;-}--rval=xfs_dir2_isblock(args,&v);-if(rval)-gotoout_free;-if(v){-rval=xfs_dir2_block_addname(args);-gotoout_free;-}--rval=xfs_dir2_isleaf(args,&v);-if(rval)-gotoout_free;-if(v)-rval=xfs_dir2_leaf_addname(args);-else-rval=xfs_dir2_node_addname(args);-out_free:-kmem_free(args);-returnrval;+returnxfs_dir_createname(tp,dp,name,0,NULL,NULL,0);}/*