From mboxrd@z Thu Jan 1 00:00:00 1970 From: "hch-jcswGhMUV9g@public.gmane.org" Subject: Re: [PATCH 12/23] sd: handle REQ_UNMAP Date: Thu, 30 Mar 2017 11:02:01 +0200 Message-ID: <20170330090201.GD12015@lst.de> References: <20170323143341.31549-1-hch@lst.de> <20170323143341.31549-13-hch@lst.de> <1490719722.2573.8.camel@sandisk.com> Mime-Version: 1.0 Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Return-path: Content-Disposition: inline In-Reply-To: <1490719722.2573.8.camel-XdAiOPVOjttBDgjK7y7TUQ@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: drbd-dev-bounces-cunTk1MwBs8qoQakbn7OcQ@public.gmane.org Errors-To: drbd-dev-bounces-cunTk1MwBs8qoQakbn7OcQ@public.gmane.org To: Bart Van Assche Cc: "axboe-tSWWG44O7X1aa/9Udqfwiw@public.gmane.org" , "linux-raid-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , "snitzer-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org" , "martin.petersen-QHcLZuEGTsvQT0dZR+AlfA@public.gmane.org" , "philipp.reisner-63ez5xqkn6DQT0dZR+AlfA@public.gmane.org" , "linux-block-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , "dm-devel-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org" , "linux-scsi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org" , "lars.ellenberg-63ez5xqkn6DQT0dZR+AlfA@public.gmane.org" , "shli-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org" , "hch-jcswGhMUV9g@public.gmane.org" , "agk-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org" , "drbd-dev-cunTk1MwBs8qoQakbn7OcQ@public.gmane.org" List-Id: dm-devel.ids On Tue, Mar 28, 2017 at 04:48:55PM +0000, Bart Van Assche wrote: > > if (sdp->no_write_same) > > return BLKPREP_INVALID; > > if (sdkp->ws16 || sector > 0xffffffff || nr_sectors > 0xffff) > = > Users can change the provisioning mode from user space from=A0SD_LBP_WS16= into > SD_LBP_WS10 so I'm not sure it's safe to skip the (sdkp->ws16 || sector > > 0xffffffff || nr_sectors > 0xffff) check if REQ_UNMAP is set. They can, and if the device has too many sectors that will already cause discard to fail, and in this case it will cause write zeroes to fail as well. The intent behind this patch is to keep the behavior the same as the old path that uses discards for zeroing. The logic looks a bit clumsy, but I'd rather keep it as-is. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Date: Thu, 30 Mar 2017 11:02:01 +0200 From: "hch@lst.de" To: Bart Van Assche Cc: "agk@redhat.com" , "lars.ellenberg@linbit.com" , "snitzer@redhat.com" , "hch@lst.de" , "martin.petersen@oracle.com" , "philipp.reisner@linbit.com" , "axboe@kernel.dk" , "shli@kernel.org" , "linux-scsi@vger.kernel.org" , "dm-devel@redhat.com" , "drbd-dev@lists.linbit.com" , "linux-block@vger.kernel.org" , "linux-raid@vger.kernel.org" Subject: Re: [PATCH 12/23] sd: handle REQ_UNMAP Message-ID: <20170330090201.GD12015@lst.de> References: <20170323143341.31549-1-hch@lst.de> <20170323143341.31549-13-hch@lst.de> <1490719722.2573.8.camel@sandisk.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 In-Reply-To: <1490719722.2573.8.camel@sandisk.com> List-ID: On Tue, Mar 28, 2017 at 04:48:55PM +0000, Bart Van Assche wrote: > > if (sdp->no_write_same) > > return BLKPREP_INVALID; > > if (sdkp->ws16 || sector > 0xffffffff || nr_sectors > 0xffff) > > Users can change the provisioning mode from user space from�SD_LBP_WS16 into > SD_LBP_WS10 so I'm not sure it's safe to skip the (sdkp->ws16 || sector > > 0xffffffff || nr_sectors > 0xffff) check if REQ_UNMAP is set. They can, and if the device has too many sectors that will already cause discard to fail, and in this case it will cause write zeroes to fail as well. The intent behind this patch is to keep the behavior the same as the old path that uses discards for zeroing. The logic looks a bit clumsy, but I'd rather keep it as-is. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from newverein.lst.de (verein.lst.de [213.95.11.211]) by mail09.linbit.com (LINBIT Mail Daemon) with ESMTP id B408E1056446 for ; Thu, 30 Mar 2017 11:02:01 +0200 (CEST) Date: Thu, 30 Mar 2017 11:02:01 +0200 From: "hch@lst.de" To: Bart Van Assche Message-ID: <20170330090201.GD12015@lst.de> References: <20170323143341.31549-1-hch@lst.de> <20170323143341.31549-13-hch@lst.de> <1490719722.2573.8.camel@sandisk.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <1490719722.2573.8.camel@sandisk.com> Cc: "axboe@kernel.dk" , "linux-raid@vger.kernel.org" , "snitzer@redhat.com" , "martin.petersen@oracle.com" , "philipp.reisner@linbit.com" , "linux-block@vger.kernel.org" , "dm-devel@redhat.com" , "linux-scsi@vger.kernel.org" , "lars.ellenberg@linbit.com" , "shli@kernel.org" , "hch@lst.de" , "agk@redhat.com" , "drbd-dev@lists.linbit.com" Subject: Re: [Drbd-dev] [PATCH 12/23] sd: handle REQ_UNMAP List-Id: "*Coordination* of development, patches, contributions -- *Questions* \(even to developers\) go to drbd-user, please." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , On Tue, Mar 28, 2017 at 04:48:55PM +0000, Bart Van Assche wrote: > > if (sdp->no_write_same) > > return BLKPREP_INVALID; > > if (sdkp->ws16 || sector > 0xffffffff || nr_sectors > 0xffff) > > Users can change the provisioning mode from user space from SD_LBP_WS16 into > SD_LBP_WS10 so I'm not sure it's safe to skip the (sdkp->ws16 || sector > > 0xffffffff || nr_sectors > 0xffff) check if REQ_UNMAP is set. They can, and if the device has too many sectors that will already cause discard to fail, and in this case it will cause write zeroes to fail as well. The intent behind this patch is to keep the behavior the same as the old path that uses discards for zeroing. The logic looks a bit clumsy, but I'd rather keep it as-is.