diff mbox

Fixup direct bi_rw modifiers

Message ID 1469915148-20452-1-git-send-email-shaun@tancheff.com (mailing list archive)
State Accepted
Headers show

Commit Message

Shaun Tancheff July 30, 2016, 9:45 p.m. UTC
bi_rw should be using bio_set_op_attrs to set bi_rw.

Signed-off-by: Shaun Tancheff <shaun@tancheff.com>

Cc: Chris Mason <clm@fb.com>
Cc: Josef Bacik <jbacik@fb.com>
Cc: David Sterba <dsterba@suse.com>
Cc: Mike Christie <mchristi@redhat.com>
---
Patch is against linux-next tag next-20160729

NOTE: In 4.7 this was not including the 'WRITE' macro so may have
      it may not have been operating as intended.
---
 fs/btrfs/extent_io.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Christoph Hellwig Aug. 1, 2016, 11:47 a.m. UTC | #1
On Sat, Jul 30, 2016 at 04:45:48PM -0500, Shaun Tancheff wrote:
> bi_rw should be using bio_set_op_attrs to set bi_rw.

Looks fine,

Reviewed-by: Christoph Hellwig <hch@lst.de>

Jens,

what do you think about renaming bi_rw?  There aren't too many users
left, and any old code that would keep using it is alsmost guranteed
to be broken, so sending a post-rc1 patch to rename it might make
everyone else life easier.  Especially as it's also grossly misnamed
now.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jens Axboe Aug. 1, 2016, 3:17 p.m. UTC | #2
On 08/01/2016 05:47 AM, Christoph Hellwig wrote:
> On Sat, Jul 30, 2016 at 04:45:48PM -0500, Shaun Tancheff wrote:
>> bi_rw should be using bio_set_op_attrs to set bi_rw.
>
> Looks fine,
>
> Reviewed-by: Christoph Hellwig <hch@lst.de>

Added, thanks Shaun.

> Jens,
>
> what do you think about renaming bi_rw?  There aren't too many users
> left, and any old code that would keep using it is alsmost guranteed
> to be broken, so sending a post-rc1 patch to rename it might make
> everyone else life easier.  Especially as it's also grossly misnamed
> now.

I was planning on doing that, after -rc1. Much better to get build
breakage, than potentially much worse breakage.
Jens Axboe Aug. 1, 2016, 7:55 p.m. UTC | #3
On 08/01/2016 09:17 AM, Jens Axboe wrote:
> On 08/01/2016 05:47 AM, Christoph Hellwig wrote:
>> On Sat, Jul 30, 2016 at 04:45:48PM -0500, Shaun Tancheff wrote:
>>> bi_rw should be using bio_set_op_attrs to set bi_rw.
>>
>> Looks fine,
>>
>> Reviewed-by: Christoph Hellwig <hch@lst.de>
>
> Added, thanks Shaun.
>
>> Jens,
>>
>> what do you think about renaming bi_rw?  There aren't too many users
>> left, and any old code that would keep using it is alsmost guranteed
>> to be broken, so sending a post-rc1 patch to rename it might make
>> everyone else life easier.  Especially as it's also grossly misnamed
>> now.
>
> I was planning on doing that, after -rc1. Much better to get build
> breakage, than potentially much worse breakage.

Set of three patches, where the target one is an actual bug fix...
Temporary branch, I'll rebase it once -rc1 is out, if more
changes/fixups need to be made in the next week until that happens.

http://git.kernel.dk/cgit/linux-block/log/?h=for-4.8/bi_rwf
Christoph Hellwig Aug. 2, 2016, 12:32 p.m. UTC | #4
On Mon, Aug 01, 2016 at 01:55:36PM -0600, Jens Axboe wrote:
> 
> Set of three patches, where the target one is an actual bug fix...
> Temporary branch, I'll rebase it once -rc1 is out, if more
> changes/fixups need to be made in the next week until that happens.
> 
> http://git.kernel.dk/cgit/linux-block/log/?h=for-4.8/bi_rwf

Thanks.  Any chance to have a slightly better name for the field?
E.g. bi_opf or bi_op_flags if we want to be a bit more verbose.
--
To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Jens Axboe Aug. 2, 2016, 6:09 p.m. UTC | #5
On 08/02/2016 05:32 AM, Christoph Hellwig wrote:
> On Mon, Aug 01, 2016 at 01:55:36PM -0600, Jens Axboe wrote:
>>
>> Set of three patches, where the target one is an actual bug fix...
>> Temporary branch, I'll rebase it once -rc1 is out, if more
>> changes/fixups need to be made in the next week until that happens.
>>
>> http://git.kernel.dk/cgit/linux-block/log/?h=for-4.8/bi_rwf
>
> Thanks.  Any chance to have a slightly better name for the field?
> E.g. bi_opf or bi_op_flags if we want to be a bit more verbose.

bi_opf is fine with me as well, don't care too strongly about bi_rwf or 
bi_opf. I don't like the longer variants.
diff mbox

Patch

diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index f67d6a1..720e6ef 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -2050,7 +2050,7 @@  int repair_io_failure(struct inode *inode, u64 start, u64 length, u64 logical,
 		return -EIO;
 	}
 	bio->bi_bdev = dev->bdev;
-	bio->bi_rw = WRITE_SYNC;
+	bio_set_op_attrs(bio, REQ_OP_WRITE, WRITE_SYNC);
 	bio_add_page(bio, page, length, pg_offset);
 
 	if (btrfsic_submit_bio_wait(bio)) {