Re: [PATCH] btrfs: Use raw numbers when setting passing uptodate parameter to btrfs_writepage_endio_finish_ordered

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 





On 3/21/19 11:25 PM, David Sterba wrote:
On Thu, Mar 21, 2019 at 04:31:53PM +0200, Nikolay Borisov wrote:


On 21.03.19 г. 16:30 ч., David Sterba wrote:
On Wed, Mar 20, 2019 at 09:53:16PM +0200, Nikolay Borisov wrote:
The uptodate parameter of btrfs_writepage_endio_finish_ordered is used
to signal whether an error has occured while writing the given page.
0 signal an error, which is propagated to callees and 1 signifies
success. In end_compressed_bio_write the ->bi_status is checked and
based on it either BLK_STS_OK (0) or BLK_STS_NOTSUPP (1) are used. While
from functional point of view this is ok it's a for the poor reader of
the code, since the block layer values are conflated with the semantics
of the parameter.

Just use plain 0 or 1. No functional changes.

Signed-off-by: Nikolay Borisov <nborisov@xxxxxxxx>
---
  fs/btrfs/compression.c | 2 +-
  1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index eb8e20b740d6..f82441f18512 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -250,7 +250,7 @@ static void end_compressed_bio_write(struct bio *bio)
  	cb->compressed_pages[0]->mapping = cb->inode->i_mapping;
  	btrfs_writepage_endio_finish_ordered(cb->compressed_pages[0],
  			cb->start, cb->start + cb->len - 1,
-			bio->bi_status ? BLK_STS_OK : BLK_STS_NOTSUPP);
+			bio->bi_status ? 0 : 1);

Essentially reverting 2dbe0c77186c691386b74391039899808a4d3f59, but
using the BLK_STS constants is definetelly wrong. The values match the
intended use of 'uptodate', but otherwise are confusing.

I actually think it can be simplified to bio->bi_staus == BLK_STS_OK.

Yes that would work, are you going to fix up the patch or shall I resend?

I'll fix it.

bio->bi_staus == BLK_STS_OK ? 1 : 0);

Is better. Its confusing. Thanks for the fix.

Reviewed-by: Anand Jain <anand.jain@xxxxxxxxxx>



[Index of Archives]     [Linux Filesystem Development]     [Linux NFS]     [Linux NILFS]     [Linux USB Devel]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]

  Powered by Linux