On Tue, Jun 30, 2020 at 07:56:46AM +0000, Damien Le Moal wrote:
On 2020/06/30 16:53, Damien Le Moal wrote:
On 2020/06/30 16:43, Kanchan Joshi wrote:
On Tue, Jun 30, 2020 at 12:37:07AM +0000, Damien Le Moal wrote:
On 2020/06/30 3:35, Kanchan Joshi wrote:
On Fri, Jun 26, 2020 at 02:50:20AM +0000, Damien Le Moal wrote:
On 2020/06/26 2:18, Kanchan Joshi wrote:
Introduce RWF_ZONE_APPEND flag to represent zone-append. User-space
sends this with write. Add IOCB_ZONE_APPEND which is set in
kiocb->ki_flags on receiving RWF_ZONE_APPEND.
Make direct IO submission path use IOCB_ZONE_APPEND to send bio with
append op. Direct IO completion returns zone-relative offset, in sector
unit, to upper layer using kiocb->ki_complete interface.
Report error if zone-append is requested on regular file or on sync
kiocb (i.e. one without ki_complete).

Signed-off-by: Kanchan Joshi <josh...@samsung.com>
Signed-off-by: SelvaKumar S <selvakuma...@samsung.com>
Signed-off-by: Arnav Dawn <a.d...@samsung.com>
Signed-off-by: Nitesh Shetty <nj.she...@samsung.com>
Signed-off-by: Javier Gonzalez <javier.g...@samsung.com>
---
 fs/block_dev.c          | 28 ++++++++++++++++++++++++----
 include/linux/fs.h      |  9 +++++++++
 include/uapi/linux/fs.h |  5 ++++-
 3 files changed, 37 insertions(+), 5 deletions(-)

diff --git a/fs/block_dev.c b/fs/block_dev.c
index 47860e5..5180268 100644
--- a/fs/block_dev.c
+++ b/fs/block_dev.c
@@ -185,6 +185,10 @@ static unsigned int dio_bio_write_op(struct kiocb *iocb)
        /* avoid the need for a I/O completion work item */
        if (iocb->ki_flags & IOCB_DSYNC)
                op |= REQ_FUA;
+
+       if (iocb->ki_flags & IOCB_ZONE_APPEND)
+               op |= REQ_OP_ZONE_APPEND;

This is wrong. REQ_OP_WRITE is already set in the declaration of "op". How can
this work ?
REQ_OP_ZONE_APPEND will override the REQ_WRITE op, while previously set op
flags (REQ_FUA etc.) will be retained. But yes, this can be made to look
cleaner.
V3 will include the other changes you pointed out. Thanks for the review.


REQ_OP_WRITE and REQ_OP_ZONE_APPEND are different bits, so there is no
"override". A well formed BIO bi_opf is one op+flags. Specifying multiple OP
codes does not make sense.

one op+flags behavior is retained here. OP is not about bits (op flags are).
Had it been, REQ_OP_WRITE (value 1) can not be differentiated from
REQ_OP_ZONE_APPEND (value 13).
We do not do "bio_op(bio) & REQ_OP_WRITE", rather we look at the
absolute value "bio_op(bio) == REQ_OP_WRITE".

Sure, the ops are not bits like the flags, but (excluding the flags) doing:

op |= REQ_OP_ZONE_APPEND;

will give you op == (REQ_OP_WRITE | REQ_OP_ZONE_APPEND). That's not what you 
want...

And yes, REQ_OP_WRITE | REQ_OP_ZONE_APPEND == REQ_OP_ZONE_APPEND... But still
not a reason for not setting the op correctly :)
Right, this is what op override was about, and intent was to minimize
the changes in the existing function. As said before, completely agree about
changing, code should not draw suspicion.


Reply via email to