Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
Message-ID: <e7d2a8ddb28dcf4cf4395630d6d0b7a04d89e560.camel@redhat.com>
Subject: Re: [PATCH] block: recalculate segment count for multi-segment
 discard requests correctly
From:   Laurence Oberman <loberman@redhat.com>
To:     Chaitanya Kulkarni <Chaitanya.Kulkarni@wdc.com>,
        Ming Lei <ming.lei@redhat.com>,
        David Jeffery <djeffery@redhat.com>
Cc:     "linux-block@vger.kernel.org" <linux-block@vger.kernel.org>,
        Jens Axboe <axboe@kernel.dk>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
Date:   Wed, 03 Feb 2021 10:08:58 -0500
In-Reply-To: <de50b6b8c867ec1f88df54f92134a99b086da8ac.camel@redhat.com>
References: <20210201164850.391332-1-djeffery@redhat.com>
         <20210202033343.GA165584@T590> <20210202204355.GA31803@redhat>
         <20210203023517.GA948998@T590>
         <BYAPR04MB4965222989DE60A2F198358286B49@BYAPR04MB4965.namprd04.prod.outlook.com>
         <de50b6b8c867ec1f88df54f92134a99b086da8ac.camel@redhat.com>
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit
Precedence: bulk

On Wed, 2021-02-03 at 08:50 -0500, Laurence Oberman wrote:
> On Wed, 2021-02-03 at 03:15 +0000, Chaitanya Kulkarni wrote:
> > On 2/2/21 18:39, Ming Lei wrote:
> > > +		/* fall through */
> > >  	case REQ_OP_WRITE_ZEROES:
> > > -		return 0;
> > 
> > I don't think returning 1 for write-zeroes is right,
> > did you test this patch with write-zeores enabled controller with
> > the right fs that triggers this behavior ?
> > 
> > 
> 
> I tested the first iteration of the patch fully mounting an XFS file
> system with -o discard and creating and deleting files.
> That was our specific RHEL8 failure we were handling here with
> David's
> first submission.
> 
> I can test his most recent, I have not done that yet.
> Again, please follow up with exactly what you want based against
> David's patch and I can test that.
> 
> Regards
> Laurence 

So if I understand what it is you were wanting I will test this now.

diff --git a/block/blk-merge.c b/block/blk-merge.c
index 808768f6b174..a9bd958c07c4 100644
--- a/block/blk-merge.c
+++ b/block/blk-merge.c
@@ -383,6 +383,13 @@ unsigned int blk_recalc_rq_segments(struct request
*rq)
        switch (bio_op(rq->bio)) {
        case REQ_OP_DISCARD:
        case REQ_OP_SECURE_ERASE:
+                if (queue_max_discard_segments(rq->q) > 1) {
+                        struct bio *bio = rq->bio;

+                        for_each_bio(bio)
+                                nr_phys_segs++;
+                        return nr_phys_segs;
+                }
+                /* fall through */
        case REQ_OP_WRITE_ZEROES:
                return 0;
        case REQ_OP_WRITE_SAME: