From: "Darrick J. Wong" <darrick.wong@oracle.com> To: Bart Van Assche <bart.vanassche@sandisk.com> Cc: axboe@kernel.dk, akpm@linux-foundation.org, linux-block@vger.kernel.org, tytso@mit.edu, martin.petersen@oracle.com, snitzer@redhat.com, linux-api@vger.kernel.org, bfoster@redhat.com, xfs@oss.sgi.com, hch@infradead.org, dm-devel@redhat.com, linux-fsdevel@vger.kernel.org Subject: Re: [PATCH 3/3] block: implement (some of) fallocate for block devices Date: Wed, 28 Sep 2016 19:09:53 -0700 [thread overview] Message-ID: <20160929020953.GV14092@birch.djwong.org> (raw) In-Reply-To: <62c7d047-166c-75e0-5b00-f2bfdcf03cb1@sandisk.com> On Wed, Sep 28, 2016 at 06:42:14PM -0700, Bart Van Assche wrote: > On 09/28/16 17:39, Darrick J. Wong wrote: > >+ if (end > isize) { > >+ if (mode & FALLOC_FL_KEEP_SIZE) { > >+ len = isize - start; > >+ end = start + len - 1; > >+ } else > >+ return -EINVAL; > >+ } > > If FALLOC_FL_KEEP_SIZE has been set and end == isize the above code won't > reduce end to isize - 1. Shouldn't "end > isize" be changed into "end >= > isize" ? Oops. Will fix and send out a v2. > >+ switch (mode) { > >+ case FALLOC_FL_ZERO_RANGE: > >+ case FALLOC_FL_ZERO_RANGE | FALLOC_FL_KEEP_SIZE: > >+ error = blkdev_issue_zeroout(bdev, start >> 9, len >> 9, > >+ GFP_KERNEL, false); > >+ if (error) > >+ return error; > >+ break; > >+ case FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE: > >+ /* Only punch if the device can do zeroing discard. */ > >+ if (!blk_queue_discard(q) || !q->limits.discard_zeroes_data) > >+ return -EOPNOTSUPP; > >+ error = blkdev_issue_discard(bdev, start >> 9, len >> 9, > >+ GFP_KERNEL, 0); > >+ if (error) > >+ return error; > >+ break; > >+ case FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE | FALLOC_FL_NO_HIDE_STALE: > >+ error = blkdev_issue_discard(bdev, start >> 9, len >> 9, > >+ GFP_KERNEL, 0); > >+ if (error) > >+ return error; > >+ break; > >+ default: > >+ return -EOPNOTSUPP; > >+ } > > Have you considered to move "if (error) return error" out of the switch > statement? Sure, I could do that. > >+ /* > >+ * Invalidate again; if someone wandered in and dirtied a page, > >+ * the caller will be given -EBUSY; > >+ */ > >+ return invalidate_inode_pages2_range(mapping, > >+ start >> PAGE_SHIFT, > >+ end >> PAGE_SHIFT); > > A comment might be appropriate here that since end is inclusive and since > the third argument of invalidate_inode_pages2_range() is inclusive that > rounding down will yield the correct result. /methot the documentation of invalidate_inode_pages2_range was clear enough on that point, but I could throw that into the comment too. --D > > Bart. >
WARNING: multiple messages have this Message-ID (diff)
From: "Darrick J. Wong" <darrick.wong@oracle.com> To: Bart Van Assche <bart.vanassche@sandisk.com> Cc: axboe@kernel.dk, hch@infradead.org, tytso@mit.edu, martin.petersen@oracle.com, snitzer@redhat.com, linux-api@vger.kernel.org, bfoster@redhat.com, xfs@oss.sgi.com, linux-block@vger.kernel.org, dm-devel@redhat.com, linux-fsdevel@vger.kernel.org, akpm@linux-foundation.org Subject: Re: [PATCH 3/3] block: implement (some of) fallocate for block devices Date: Wed, 28 Sep 2016 19:09:53 -0700 [thread overview] Message-ID: <20160929020953.GV14092@birch.djwong.org> (raw) In-Reply-To: <62c7d047-166c-75e0-5b00-f2bfdcf03cb1@sandisk.com> On Wed, Sep 28, 2016 at 06:42:14PM -0700, Bart Van Assche wrote: > On 09/28/16 17:39, Darrick J. Wong wrote: > >+ if (end > isize) { > >+ if (mode & FALLOC_FL_KEEP_SIZE) { > >+ len = isize - start; > >+ end = start + len - 1; > >+ } else > >+ return -EINVAL; > >+ } > > If FALLOC_FL_KEEP_SIZE has been set and end == isize the above code won't > reduce end to isize - 1. Shouldn't "end > isize" be changed into "end >= > isize" ? Oops. Will fix and send out a v2. > >+ switch (mode) { > >+ case FALLOC_FL_ZERO_RANGE: > >+ case FALLOC_FL_ZERO_RANGE | FALLOC_FL_KEEP_SIZE: > >+ error = blkdev_issue_zeroout(bdev, start >> 9, len >> 9, > >+ GFP_KERNEL, false); > >+ if (error) > >+ return error; > >+ break; > >+ case FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE: > >+ /* Only punch if the device can do zeroing discard. */ > >+ if (!blk_queue_discard(q) || !q->limits.discard_zeroes_data) > >+ return -EOPNOTSUPP; > >+ error = blkdev_issue_discard(bdev, start >> 9, len >> 9, > >+ GFP_KERNEL, 0); > >+ if (error) > >+ return error; > >+ break; > >+ case FALLOC_FL_PUNCH_HOLE | FALLOC_FL_KEEP_SIZE | FALLOC_FL_NO_HIDE_STALE: > >+ error = blkdev_issue_discard(bdev, start >> 9, len >> 9, > >+ GFP_KERNEL, 0); > >+ if (error) > >+ return error; > >+ break; > >+ default: > >+ return -EOPNOTSUPP; > >+ } > > Have you considered to move "if (error) return error" out of the switch > statement? Sure, I could do that. > >+ /* > >+ * Invalidate again; if someone wandered in and dirtied a page, > >+ * the caller will be given -EBUSY; > >+ */ > >+ return invalidate_inode_pages2_range(mapping, > >+ start >> PAGE_SHIFT, > >+ end >> PAGE_SHIFT); > > A comment might be appropriate here that since end is inclusive and since > the third argument of invalidate_inode_pages2_range() is inclusive that > rounding down will yield the correct result. /methot the documentation of invalidate_inode_pages2_range was clear enough on that point, but I could throw that into the comment too. --D > > Bart. > _______________________________________________ xfs mailing list xfs@oss.sgi.com http://oss.sgi.com/mailman/listinfo/xfs
next prev parent reply other threads:[~2016-09-29 2:09 UTC|newest] Thread overview: 66+ messages / expand[flat|nested] mbox.gz Atom feed top 2016-09-29 0:39 [PATCH v10 0/3] fallocate for block devices Darrick J. Wong 2016-09-29 0:39 ` Darrick J. Wong 2016-09-29 0:39 ` [PATCH 1/3] block: invalidate the page cache when issuing BLKZEROOUT Darrick J. Wong 2016-09-29 0:39 ` Darrick J. Wong 2016-09-29 0:39 ` Darrick J. Wong 2016-09-29 1:16 ` Bart Van Assche 2016-09-29 1:16 ` Bart Van Assche 2016-09-29 5:56 ` Hannes Reinecke 2016-09-29 5:56 ` Hannes Reinecke 2016-09-29 0:39 ` [PATCH 2/3] block: require write_same and discard requests align to logical block size Darrick J. Wong 2016-09-29 0:39 ` Darrick J. Wong 2016-09-29 5:56 ` Hannes Reinecke 2016-09-29 5:56 ` Hannes Reinecke 2016-09-29 5:56 ` Hannes Reinecke 2016-09-29 0:39 ` [PATCH 3/3] block: implement (some of) fallocate for block devices Darrick J. Wong 2016-09-29 0:39 ` Darrick J. Wong 2016-09-29 1:42 ` Bart Van Assche 2016-09-29 1:42 ` Bart Van Assche 2016-09-29 1:42 ` Bart Van Assche 2016-09-29 2:09 ` Darrick J. Wong [this message] 2016-09-29 2:09 ` Darrick J. Wong 2016-09-29 2:19 ` [PATCH v2 " Darrick J. Wong 2016-09-29 2:19 ` Darrick J. Wong 2016-09-29 20:08 ` Bart Van Assche 2016-09-29 20:08 ` Bart Van Assche 2016-09-29 20:08 ` Bart Van Assche 2016-09-29 20:35 ` Darrick J. Wong 2016-09-29 20:35 ` Darrick J. Wong 2016-09-29 5:57 ` [PATCH " Hannes Reinecke 2016-09-29 5:57 ` Hannes Reinecke -- strict thread matches above, loose matches on Subject: below -- 2016-09-29 21:16 [PATCH v11 0/3] " Darrick J. Wong 2016-09-29 21:16 ` [PATCH 3/3] block: implement (some of) " Darrick J. Wong 2016-09-29 21:16 ` Darrick J. Wong 2016-08-26 0:02 [PATCH v10 0/3] " Darrick J. Wong 2016-08-26 0:02 ` [PATCH 3/3] block: implement (some of) " Darrick J. Wong 2016-08-26 0:02 ` Darrick J. Wong 2016-06-17 1:17 [PATCH v9 0/3] " Darrick J. Wong 2016-06-17 1:17 ` [PATCH 3/3] block: implement (some of) " Darrick J. Wong 2016-06-17 1:17 ` Darrick J. Wong 2016-06-17 1:17 ` Darrick J. Wong 2016-04-13 4:01 [RFC DONOTMERGE v8 0/3] " Darrick J. Wong 2016-04-13 4:01 ` [PATCH 3/3] block: implement (some of) " Darrick J. Wong 2016-04-13 4:01 ` Darrick J. Wong 2016-04-13 4:01 ` Darrick J. Wong 2016-03-15 19:42 [PATCH v7 0/3] fallocate for block devices to provide zero-out Darrick J. Wong 2016-03-15 19:42 ` [PATCH 3/3] block: implement (some of) fallocate for block devices Darrick J. Wong 2016-03-15 19:42 ` Darrick J. Wong 2016-03-21 15:38 ` Christoph Hellwig 2016-03-21 15:38 ` Christoph Hellwig 2016-03-21 17:52 ` Darrick J. Wong 2016-03-21 17:52 ` Darrick J. Wong 2016-03-21 18:17 ` Christoph Hellwig 2016-03-21 18:17 ` Christoph Hellwig 2016-03-21 18:21 ` Martin K. Petersen 2016-03-21 18:21 ` Martin K. Petersen 2016-03-21 18:52 ` Mike Snitzer 2016-03-21 18:52 ` Mike Snitzer 2016-03-21 19:11 ` Darrick J. Wong 2016-03-21 19:11 ` Darrick J. Wong 2016-03-21 19:22 ` Mike Snitzer 2016-03-21 20:59 ` Brian Foster 2016-03-21 20:59 ` Brian Foster 2016-03-05 0:55 [PATCH v6 0/3] fallocate for block devices to provide zero-out Darrick J. Wong 2016-03-05 0:56 ` [PATCH 3/3] block: implement (some of) fallocate for block devices Darrick J. Wong 2016-03-05 3:06 ` Linus Torvalds 2016-03-05 3:06 ` Linus Torvalds 2016-03-05 20:57 ` Christoph Hellwig 2016-03-05 20:57 ` Christoph Hellwig 2016-03-05 3:13 ` Linus Torvalds 2016-03-05 3:13 ` Linus Torvalds 2016-03-05 20:58 ` Christoph Hellwig 2016-03-05 20:58 ` Christoph Hellwig
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20160929020953.GV14092@birch.djwong.org \ --to=darrick.wong@oracle.com \ --cc=akpm@linux-foundation.org \ --cc=axboe@kernel.dk \ --cc=bart.vanassche@sandisk.com \ --cc=bfoster@redhat.com \ --cc=dm-devel@redhat.com \ --cc=hch@infradead.org \ --cc=linux-api@vger.kernel.org \ --cc=linux-block@vger.kernel.org \ --cc=linux-fsdevel@vger.kernel.org \ --cc=martin.petersen@oracle.com \ --cc=snitzer@redhat.com \ --cc=tytso@mit.edu \ --cc=xfs@oss.sgi.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
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.