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;
IronPort-SDR: qwWxINDWcah5dH7YjLiELABEW8iVEUFMil4yrnmTzUvUDqyyw8/vlhhn5O86D8G7ihlpp/kfPg
 sTmMAZM3AjkWQc/+55/81TvUc9yyGUR04+chcTvO4gZMhmyI7KyH5KbCpST/5JjX1/X+Y3eQd1
 cH9x9FDhugoEyU9rUBTHGc66sUgWTxeRP6fS/kRACAQ13O/pcurzDN6uVMFohWtVIPf1ocYLp2
 sOQJIN3Qn6oQAh4tCoB9F/qIUWH/LrQi1j/2jKr4mxvtk9Nhv/sfFC81eqMY1IPttYQ1zKPccE
 q38=
From:   Damien Le Moal <Damien.LeMoal@wdc.com>
To:     Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>
CC:     "stable@vger.kernel.org" <stable@vger.kernel.org>,
        Kanchan Joshi <joshi.k@samsung.com>,
        Jens Axboe <axboe@kernel.dk>
Subject: Re: [PATCH 5.9 374/391] null_blk: synchronization fix for zoned
 device
Thread-Topic: [PATCH 5.9 374/391] null_blk: synchronization fix for zoned
 device
Thread-Index: AQHWsiM76BgMC3WbRkyxRh5dMkktVg==
Date:   Tue, 3 Nov 2020 23:36:35 +0000
Message-ID: <BL0PR04MB65147F17F20E4E4A77E35B38E7110@BL0PR04MB6514.namprd04.prod.outlook.com>
References: <20201103203348.153465465@linuxfoundation.org>
 <20201103203412.385651316@linuxfoundation.org>
Accept-Language: en-US
Content-Language: en-US
wdcipoutbound: EOP-TRUE
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-MS-Exchange-CrossTenant-Network-Message-Id: 028674da-f223-4a1b-5034-08d880514d77
X-MS-Exchange-CrossTenant-originalarrivaltime: 03 Nov 2020 23:36:35.2007
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: b61c8803-16f3-4c35-9b17-6f65f441df86
X-MS-Exchange-CrossTenant-mailboxtype: HOSTED
X-MS-Exchange-CrossTenant-userprincipalname: BWWvrv2p5Qp9Vwq32s00lkVNB5PLCpeQR9mRY9ZUVzvQcIurXRkZVrfKZb7fOI5Ma8jMNv0EbA2EFI415qV9Kg==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: MN2PR04MB6445
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 2020/11/04 5:52, Greg Kroah-Hartman wrote:=0A=
> From: Kanchan Joshi <joshi.k@samsung.com>=0A=
> =0A=
> commit 35bc10b2eafbb701064b94f283b77c54d3304842 upstream.=0A=
> =0A=
> Parallel write,read,zone-mgmt operations accessing/altering zone state=0A=
> and write-pointer may get into race. Avoid the situation by using a new=
=0A=
> spinlock for zoned device.=0A=
> Concurrent zone-appends (on a zone) returning same write-pointer issue=0A=
> is also avoided using this lock.=0A=
> =0A=
> Cc: stable@vger.kernel.org=0A=
> Fixes: e0489ed5daeb ("null_blk: Support REQ_OP_ZONE_APPEND")=0A=
> Signed-off-by: Kanchan Joshi <joshi.k@samsung.com>=0A=
> Reviewed-by: Damien Le Moal <damien.lemoal@wdc.com>=0A=
> Signed-off-by: Jens Axboe <axboe@kernel.dk>=0A=
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>=0A=
=0A=
Greg,=0A=
=0A=
I sent a followup patch fixing a bug introduced by this patch, but I forgot=
 to=0A=
mark it for stable. The patch is=0A=
=0A=
commit aa1c09cb65e2 "null_blk: Fix locking in zoned mode"=0A=
=0A=
Could you pickup that one too please ?=0A=
=0A=
Best regards.=0A=
=0A=
>=0A=
> ---=0A=
>  drivers/block/null_blk.h       |    1 +=0A=
>  drivers/block/null_blk_zoned.c |   22 ++++++++++++++++++----=0A=
>  2 files changed, 19 insertions(+), 4 deletions(-)=0A=
> =0A=
> --- a/drivers/block/null_blk.h=0A=
> +++ b/drivers/block/null_blk.h=0A=
> @@ -44,6 +44,7 @@ struct nullb_device {=0A=
>  	unsigned int nr_zones;=0A=
>  	struct blk_zone *zones;=0A=
>  	sector_t zone_size_sects;=0A=
> +	spinlock_t zone_lock;=0A=
>  =0A=
>  	unsigned long size; /* device size in MB */=0A=
>  	unsigned long completion_nsec; /* time in ns to complete a request */=
=0A=
> --- a/drivers/block/null_blk_zoned.c=0A=
> +++ b/drivers/block/null_blk_zoned.c=0A=
> @@ -45,6 +45,7 @@ int null_init_zoned_dev(struct nullb_dev=0A=
>  	if (!dev->zones)=0A=
>  		return -ENOMEM;=0A=
>  =0A=
> +	spin_lock_init(&dev->zone_lock);=0A=
>  	if (dev->zone_nr_conv >=3D dev->nr_zones) {=0A=
>  		dev->zone_nr_conv =3D dev->nr_zones - 1;=0A=
>  		pr_info("changed the number of conventional zones to %u",=0A=
> @@ -131,8 +132,11 @@ int null_report_zones(struct gendisk *di=0A=
>  		 * So use a local copy to avoid corruption of the device zone=0A=
>  		 * array.=0A=
>  		 */=0A=
> +		spin_lock_irq(&dev->zone_lock);=0A=
>  		memcpy(&zone, &dev->zones[first_zone + i],=0A=
>  		       sizeof(struct blk_zone));=0A=
> +		spin_unlock_irq(&dev->zone_lock);=0A=
> +=0A=
>  		error =3D cb(&zone, i, data);=0A=
>  		if (error)=0A=
>  			return error;=0A=
> @@ -277,18 +281,28 @@ static blk_status_t null_zone_mgmt(struc=0A=
>  blk_status_t null_process_zoned_cmd(struct nullb_cmd *cmd, enum req_opf =
op,=0A=
>  				    sector_t sector, sector_t nr_sectors)=0A=
>  {=0A=
> +	blk_status_t sts;=0A=
> +	struct nullb_device *dev =3D cmd->nq->dev;=0A=
> +=0A=
> +	spin_lock_irq(&dev->zone_lock);=0A=
>  	switch (op) {=0A=
>  	case REQ_OP_WRITE:=0A=
> -		return null_zone_write(cmd, sector, nr_sectors, false);=0A=
> +		sts =3D null_zone_write(cmd, sector, nr_sectors, false);=0A=
> +		break;=0A=
>  	case REQ_OP_ZONE_APPEND:=0A=
> -		return null_zone_write(cmd, sector, nr_sectors, true);=0A=
> +		sts =3D null_zone_write(cmd, sector, nr_sectors, true);=0A=
> +		break;=0A=
>  	case REQ_OP_ZONE_RESET:=0A=
>  	case REQ_OP_ZONE_RESET_ALL:=0A=
>  	case REQ_OP_ZONE_OPEN:=0A=
>  	case REQ_OP_ZONE_CLOSE:=0A=
>  	case REQ_OP_ZONE_FINISH:=0A=
> -		return null_zone_mgmt(cmd, op, sector);=0A=
> +		sts =3D null_zone_mgmt(cmd, op, sector);=0A=
> +		break;=0A=
>  	default:=0A=
> -		return null_process_cmd(cmd, op, sector, nr_sectors);=0A=
> +		sts =3D null_process_cmd(cmd, op, sector, nr_sectors);=0A=
>  	}=0A=
> +	spin_unlock_irq(&dev->zone_lock);=0A=
> +=0A=
> +	return sts;=0A=
>  }=0A=
> =0A=
> =0A=
> =0A=
=0A=
=0A=
-- =0A=
Damien Le Moal=0A=
Western Digital Research=0A=