On 9/27/22 5:10 PM, Damien Le Moal wrote: > On 9/28/22 08:07, Damien Le Moal wrote: >> On 9/28/22 01:52, Jens Axboe wrote: >>> On 9/27/22 10:51 AM, Christoph Hellwig wrote: >>>> On Tue, Sep 27, 2022 at 10:04:19AM -0600, Jens Axboe wrote: >>>>> Ah yes, good point. We used to have this notion of 'fs' request, don't >>>>> think we do anymore. Because it really should just be: >>>> >>>> A fs request is a !passthrough request. >>> >>> Right, that's the condition I made below too. >>> >>>>> if (zoned && (op & REQ_OP_WRITE) && fs_request) >>>>> return NULL; >>>>> >>>>> for that condition imho. I guess we could make it: >>>>> >>>>> if (zoned && (op & REQ_OP_WRITE) && !(op & REQ_OP_DRV_OUT)) >>>>> return NULL; >>>> >>>> Well, the only opcodes we do zone locking for is REQ_OP_WRITE and >>>> REQ_OP_WRITE_ZEROES. So this should be: >>>> >>>> if (zoned && (op == REQ_OP_WRITE || op == REQ_OP_WRITE_ZEROES)) >>>> return NULL; >>> >>> I'd rather just make it explicit and use that. Pankaj, do you want >>> to spin a v2 with that? >> >> It would be nice to reuse the bio equivalent of >> blk_req_needs_zone_write_lock(). >> >> The test would be: >> >> if (bio_needs_zone_write_locking()) >> return NULL; > > Note that we could also add a "IS_ENABLED(CONFIG_BLK_DEV_ZONED) &&" to > the condition or stub the helper to have this hunk disappear for the > !CONFIG_BLK_DEV_ZONED case. Indeed, that would be nice. -- Jens Axboe
