From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1756934AbcHWBZa (ORCPT ); Mon, 22 Aug 2016 21:25:30 -0400 Received: from esa1.hgst.iphmx.com ([68.232.141.245]:45386 "EHLO esa1.hgst.iphmx.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1755306AbcHWBZ1 (ORCPT ); Mon, 22 Aug 2016 21:25:27 -0400 X-IronPort-AV: E=Sophos;i="5.28,563,1464624000"; d="scan'";a="15021724" Authentication-Results: spf=none (sender IP is ) smtp.mailfrom=Damien.LeMoal@hgst.com; Subject: Re: [PATCH v2 2/4] On Discard either do Reset WP or Write Same To: Shaun Tancheff References: <20160822043116.21168-1-shaun@tancheff.com> <20160822043116.21168-3-shaun@tancheff.com> <53c2949f-f8b9-463f-2adf-faf4603429bb@hgst.com> CC: Shaun Tancheff , , , LKML , Jens Axboe , Christoph Hellwig , "James E . J . Bottomley" , "Martin K . Petersen" , Hannes Reinecke , Josh Bingaman , Dan Williams , Sagi Grimberg , Mike Christie , Toshi Kani , Ming Lei From: Damien Le Moal Message-ID: Date: Tue, 23 Aug 2016 10:25:02 +0900 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.11; rv:45.0) Gecko/20100101 Thunderbird/45.2.0 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit X-Originating-IP: [199.255.47.8] X-ClientProxiedBy: CO2PR11CA0030.namprd11.prod.outlook.com (10.141.242.168) To BY2PR04MB1973.namprd04.prod.outlook.com (10.166.111.13) X-MS-Office365-Filtering-Correlation-Id: 5444e4ac-b864-4a82-23f9-08d3caf45322 X-Microsoft-Exchange-Diagnostics: 1;BY2PR04MB1973;2:68+xmkoUThnIf37G2KJG2izdYMS3PDSEiTlLFfPt1d1mr7jhqzkqxRZK5bbJAeUFj/2tRPxWtJkhoxrez38RhZPVwjc1uT9UzoPNZspOZ9r0VabHVIU02Ohxor+K71PhHNZbV4FQ2DhBw+o1ueyMVwAb7D0xiZXdFROSwTqqo8rv61GhW6cHmRN81YMMUfXZ;3:wv/B5PBE72uYWtpQv8xf1Jrn/6bWCmtbSgiZT2Tv8fhB+a8IjanByvNlpgkWsSDsgi8+bmun1eCAp0CUu5ko33vYj438UaPW4UyT32YemRPdIXsYxQmL7MqX+lxL2NqS X-Microsoft-Antispam: UriScan:;BCL:0;PCL:0;RULEID:;SRVR:BY2PR04MB1973; X-Microsoft-Exchange-Diagnostics: 1;BY2PR04MB1973;25:JiGSbEgh7gmjLy8j54WT6gIHxIARPCpghAJqG9JZ7c7dP/rn1kQYU2GaPFAJ5S8YgkVkaCkT2kvaa0/icN6NsGnNg+Y2YtyyuFHQTcGzGiKw8VHfy/u2Qyz0rLTlt4WYoC6VrahiMnTLH4njXzrrHwKL2R5vcehYo8SYFEo6ciMQ83G1q0BOLVpFPFOr+/i0n0wTn62koz0RjPNwGLIoe3OqHH5Y1RQTm0KPdu661BdOQfA2U4sE7OGH/Mvv1b0oS55lGrcgZTyNFFQ5LjREs0XftGfALKbaBEp953fvb7GPAT+BDLveyRFvTOURBwGC/MQv5Q7+7QH2AEGotmDtujiTSXygTgVGuSaz73AYnd1Jz6ItgKGIYkiCSoTTJ8tU+EIU0OkxJR/PG7DSSfHlaWhTp8aN+XLRKDLLPn71BXDYrFjvhDxiPwXf9TplBxGuANpU4OwPirjVwrbx1f/kilh7leoxZcTh0zPR26a2UERMs8BqAfEDsmH+vKTkLtFlxlG9cAXBQJj0V5YgP4QCfPan6IbPnkkuCHr4/QrtMiKGhDNba2CjBvjBG9YkDIqX+fDVwxIRGIyK3qchjEquX2Wx61oHgpWYud+VqgDv0uG1EQSanA0r2mCDwxDsrYk3+Asg1Et7JKz00yf1G8qDjE49HbapiUug9P1c4xBSoo9l2mEKt7o3AMLCQ6oJeohuunJvch6GcRyrFwrjQtWeqXSUGhURNRsURzcqUk3OVh5Glop4ft7q9GpUDyi3No9goBTnHlCQ27nFXqSIzFkFXzD0zDDMNJO7n7AabTmJ9OI= X-Microsoft-Exchange-Diagnostics: 1;BY2PR04MB1973;31:7csmYV0AbL6PCVGCPI3+PEg/uWrSlis+qW/f1n9PZ2d8YbNOuD8xlCHgaiWlFB1OMuerN+cpUgn3n7thuZZuLaZxM639FyHwqSU04Vb8nhZKnDgYY+dCskahpF63GyvBJ6xMeKAyKeJwamMiqZg3PbXToEN6Puajb5xuqyzndQ41vZu2p8MYPI1Rufukc7CiV8W2eVU0GT0OzB6Tmsr3k3D+h6wm74EkLtLxdAHBBdw=;20:pePJnAMCCGLFrW5udz6IiPgb3z/g2cbspjg+ux99GHi8gENeuPMuzG5aVtzyYbcRRZEqfaBMYvNs0nOhTvrnXOVYbV3qJoMbIK/JDKbPDEdCSirP3nvGl4kYworXveG9pFp+7XCy4MiTY6wFR/epdGTXs2UR4I/FK/86jWh85IZAznk8qtonXvRigl6GEttt4quE5NQTfDTBekrW6WoSsUNCsyItQkq1KV0xoxdFGmKySkBQ5QGfsLvGCW/X3Of1+qYGkaqmqohW1GukZ9z37vy972LcDb3a0o5SeCDqalwycAGWfkz9iGHAo3c4cRV8nwalKUupOUpOu3M7mHQpdKslKmupGsqcpm3Luwt1SiuGIXTUnj8ItOOle/CkQZMPe8kMyL5xEkZZlha1M8LHHqbI0fFjWdKFVogZPTyS10z2bLrIA6GSB5fiqk6yYwG/94UjNBOgfBEJ8/v1OG7sFDlQudr5HxGwcPb7IEjMBWIBoe1NgzF5V4Nv0v3NfPX4 X-Microsoft-Antispam-PRVS: X-Exchange-Antispam-Report-Test: UriScan:(79135771888625)(56741022784758); X-Exchange-Antispam-Report-CFA-Test: BCL:0;PCL:0;RULEID:(6040176)(601004)(2401047)(5005006)(8121501046)(3002001)(10201501046)(6055026);SRVR:BY2PR04MB1973;BCL:0;PCL:0;RULEID:;SRVR:BY2PR04MB1973; X-Microsoft-Exchange-Diagnostics: 1;BY2PR04MB1973;4:Dz+CREaA0Kzs1Yz+ePyTOd1TSMIlGDD0CLXPSyqOHfYj/tucJKsIwdStKmvuhv3yNCyg0d6HXvD/Jbq89344bRVWtKNNNGpmHhonnLJbBLXRfOYrOn8JnMd7YT2K7RnwUHfsganEFcDKaAD9QXxdv6F4FdDRRPMaF9dcJUeBrIc372ySFOYzuyQR0p++65UlI33RRL2bRfVjQJM8R1ydjVnYvUF4+xJ4inR9BeGBI1qvksz8c47WLIiF7YXK2zJpVoYaAexiVqYmRVbRbKj4SZpwRYi9xQPRlxfaDGYDhnzHxta6GUWAvy7LwxVauV7xIDDshELr1b01TqCTuOEjHWROvsDKg3zkKFNxCOpL1DFIDVxBV1rLyOxLfHPjfk8BgiteXRM1dc2oOcr6BO2EjOFin1aRkcMXq4id3DSSEErZ/gy2dewQuH2XIWujEj4aWVdegwQ6sKyLeUQ1t5QtS9bRLqUJXspm+SJcP31sUMw= X-Forefront-PRVS: 004395A01C X-Forefront-Antispam-Report: SFV:NSPM;SFS:(10019020)(4630300001)(6009001)(7916002)(377454003)(189002)(51444003)(199003)(24454002)(377424004)(46034005)(54356999)(76176999)(50986999)(7416002)(83506001)(33646002)(77096005)(68736007)(31686004)(42186005)(19580405001)(4001350100001)(106356001)(230700001)(93886004)(31696002)(6116002)(586003)(101416001)(189998001)(110136002)(86362001)(105586002)(19580395003)(65806001)(47776003)(5660300001)(97736004)(92566002)(50466002)(3846002)(65956001)(81166006)(8676002)(81156014)(66066001)(64126003)(305945005)(4326007)(23676002)(15974865002)(2950100001)(36756003)(7846002)(2906002)(7736002)(7099028)(65826006)(18886075002);DIR:OUT;SFP:1102;SCL:1;SRVR:BY2PR04MB1973;H:damien-imac.dhcp.fujisawa.hgst.com;FPR:;SPF:None;PTR:InfoNoRecords;MX:1;A:1;LANG:en; X-Microsoft-Exchange-Diagnostics: =?utf-8?B?MTtCWTJQUjA0TUIxOTczOzIzOnVCWTVtblFhcjJIdUFZUm1YWDRWMHJzRktW?= =?utf-8?B?cWtNaWtuOUdFK09lbkZLSnBuaExTbDBqb0FFOTN0dWxqQkJsSE44bXZrNXkx?= =?utf-8?B?OGRscGk0amk0ZjQ3SnJjMGhaUnlDQ1pLYlFOcWVkUmZZclpDSHM1cHRoVDdu?= =?utf-8?B?T3JrMWdQQTg0Q0hBczVnVlJMNTI1cjlMYVZTUWgwbm01Zm9oeFBMYklndGk1?= =?utf-8?B?U2NSb2luSy81WW5xelNoOHl6QmNxbGVxZG9iZExGcWdXb3NwdmdVQzBjTXpy?= =?utf-8?B?aGpDS1dNaWNhU2kvV0FhdXkzQ0tsc1Y3amNNZFdIS0JmN0xMZTcwMTBsUFVu?= =?utf-8?B?a1haTDhHNUg0REZaem5EMXIyMldLY1pzYXU2YVBSV1Q0SUZBS1ZRZHlma0VL?= =?utf-8?B?NFBLZ2xSMmNJTGRqZ3ZNVkdoZTNXbisxTDJIUUh1SlBIZ0cxYW5EQlhrZWxZ?= =?utf-8?B?K3lLdmxJVE5xKzhOMHpYeVZldXQ2MzBUSCtnQy92akhjRXgwVjhOZmRqeVFq?= =?utf-8?B?UVd5b3o5SmVWWWcyVW5mODJORW5rR1FkTGNxbFg3eTQwaGNyblpiRWJTME5H?= =?utf-8?B?dFRuOUhCNExxbTQvU0ZCZHBnY0tUQzdWd2FWUFVkOEk5cWNYTnlMUkZaem5S?= =?utf-8?B?SStTMkpGUWk2aTllb0JnNGZVenoxZHRlRWVaemVwQ1J3T3k1THhVZmk5SDBl?= =?utf-8?B?N3c4dG1Na1VRemFscmtDc3J3SE8rN1ZNSzRMNFkvb3k0S1hwTjYwU0VHUTFB?= =?utf-8?B?SVFkamV3V1lWUlM4Vmt1VCtjdnQ5YjlobG4wWVhLdDVxWWxJRDVJNmI2SnZO?= =?utf-8?B?bUFPclViZ1JuaEdUUUhKeEt0bkNSVm1nc1VBZ1hoM1k4OUdNMnJCRVE0cFZQ?= =?utf-8?B?NWJmTGdNSGNETzFybDJPdEx3dGFUczVEcXVQQlN3N0U5Vy9vd3RHb2ZXN2R2?= =?utf-8?B?VXhyS1VVeS9QZ0RPc2owZUNzYWtuZWdPQ1Z3T1pBSHp2azRPK0k1YTVXaUpz?= =?utf-8?B?TnE5ak5wTzJLaWJxVFVvZHBpY2pPZzRueTQvQUlabXVObnE3cU92U3B1bm5M?= =?utf-8?B?L1d2UHJHNVJheUJyK05JQ3B3czJzOC9GWXZ6OTdad3llYmxxdU5qOExZZXRj?= =?utf-8?B?c3NWS1Jiay8rNlYxVmdPSTQ1MVdtYVB1czh6UlYvcUtrelVuTEV3U0ZRemN3?= =?utf-8?B?QTFKM2Zpb3VhUC9naDV2NUtVbEpwVnY0OW94bkd2QkhqVXpxMUFiS2MxdFpK?= =?utf-8?B?VmpGL1Y5MzNDYmtZZlZoOHV6b3Z2K1dyQ2JjYkt2cnkzaVphd1FNZk1ENjZ0?= =?utf-8?B?a3JoL0c0WDB4QkdqOERaeUs0V1JveW9sQk50ZUF1RnRWRkk5M2V6bERMb2N3?= =?utf-8?B?YWdiTC96UG1HL3JxVlc4ZTV1U3lYcHhFOE53T1UrbWdEYjZCWW45Q1BVcDJU?= =?utf-8?B?bWxEV2RIcmg2bzAwTDJXbmZwQUJvYnIrN3N5cUI4RFJmOHhSWEJIQnFaOWxK?= =?utf-8?B?N2hLSFRqNUN5YWlIbXlPMDZRd3hCa1lFTDVNOFFtTUo3aVVMRWlTTWw4YkZv?= =?utf-8?B?RnB6SmNVRnVMWXRtM1JxN09Oai9XUS90dTkwZWhzY25ySVhHZmJMUzlWM29q?= =?utf-8?B?S29HNXQrUkhWbmM2VlNrcGs1L1RBdkgwSDZDRTQwVTJ5d1VRY00yOU1PVTdo?= =?utf-8?B?SkVQZjVOaXoyMmhzTkhYcC9hVjZ1cjVmRzh3NU1HOEhOWVo0Sk00SVlFVWdN?= =?utf-8?B?R3hqWTRMdHM3Znc5RmtVRE43bUkzUEQwaXhIU2hDKzV0UHlhUEkzdjhVM2lT?= =?utf-8?B?Q25Nd0J5NGJnejFKWmFza3Nhd2pKQUREbjNzUXJYU091aWlzWlkwZ0tTZVJq?= =?utf-8?B?VHVBUVhVb29SdUZSY01SNkxYQURKMUhYbC9CcWVRL0hQWWF0SHRxMDY2dThO?= =?utf-8?B?TEdPemsrQnJBPT0=?= X-Microsoft-Exchange-Diagnostics: 1;BY2PR04MB1973;6:BIppSO4Pd5a85OxNXalcOI6uQoL/o/ckEiO6y5NyMo89OovUrp9klKaSRd9+vhQCIubfktKDiAKMJg2Mzslt5T6dxUZEzjiMK/iNPVGCedXHiaOgF2/OT0VpCC36xr0HpLl+hwjSJTEo0NnFVgRStJyS+OjR3VBsbyl/KlJzHkF+9qi/QJrUwVKvLQy269wdB5yR3GGhQbDEYRd5N7R3dLUBjIvOcMIypcovMfP6fwjUS+wLZwEFBKz93vGCvbWcj7MGQ2hlb3fqn1IYxYdH4WcKX6La+piV2m5CoXOKN/MGhcHd+so3RriG8QhN8mgS5rh61fs/jEj99px3y8is3A==;5:gustp25AtDhQgNx5i9+Y+mwhhRWgT/Td8GDwdWiPbam81M2WA0zvR6p5K3L00xkxGMbPKEf2UK+8MSHBiCQ387EuDfy5PrKgaPrn4cl8Q2QfTHISyHnsadsaK5GudVnLOOVx5SVtY6dArbbbF8HWfA==;24:WzABU3isl/f5tsVHl+fpuHlRy3YFmXh6k7jKbrkkD5TF57YeVn1i5XEwj8+8C95FVWXjxY/uahB+nK8SLyzM4kM2BwKprpScPX7+cPBSXiE=;7:6cwjapExvVraEkd/Xi6hSEwrBt3hoWzUWdkNG18HCNB0/fJ5gdmXSQJduQyCraGwsFor9AVvU6DBqy++8kKQEbCYkNucS4jyy1rRUx1v+stCFzHe0XbTTAeDKQ8O3j4cEE3M522F/NhQN97wYsEPuas4QrkV5zgrlum9gtZ0AyHylysHaevqRNumdOv88dhWDuuKuRGHaODhTK/zd7hZO9pHZFwE4Z8d+ZNjUMYsDXdfObXVLm/+II1HWrN4RAug SpamDiagnosticOutput: 1:99 SpamDiagnosticMetadata: NSPM X-Microsoft-Exchange-Diagnostics: 1;BY2PR04MB1973;20:ZhzdiLJ7h0QBFnBNu/Dhh2vj6pxTMZGAOYFu0R8OuFInQ0x3F9eKJG2as3n3By3kxWrBpOq6AcY+IvtgGCb2MMGyNECam3sWoIFQR4DyhcbDN4hmjTnqdsEudc5C4wzJUZqFtTK30OvrFtGZUpeiwVtcN+bsbuanY2sBw4ma1jf4l8YxReTk23MsbfE+tp3Rn+e+mCtNAVhH3D3IxXx5p2Ttb4r9OoQAMJi9+vEN7XKAw0vqWScs4OUjtk5XIx+H X-OriginatorOrg: hgst.com X-MS-Exchange-CrossTenant-OriginalArrivalTime: 23 Aug 2016 01:25:07.8668 (UTC) X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY2PR04MB1973 Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Shaun, On 8/23/16 09:22, Shaun Tancheff wrote: > On Mon, Aug 22, 2016 at 6:57 PM, Damien Le Moal wrote: >> >> Shaun, >> >> On 8/22/16 13:31, Shaun Tancheff wrote: >> [...] >>> -int sd_zbc_setup_discard(struct scsi_disk *sdkp, struct request *rq, >>> - sector_t sector, unsigned int num_sectors) >>> +int sd_zbc_setup_discard(struct scsi_cmnd *cmd) >>> { >>> - struct blk_zone *zone; >>> + struct request *rq = cmd->request; >>> + struct scsi_device *sdp = cmd->device; >>> + struct scsi_disk *sdkp = scsi_disk(rq->rq_disk); >>> + sector_t sector = blk_rq_pos(rq); >>> + unsigned int nr_sectors = blk_rq_sectors(rq); >>> int ret = BLKPREP_OK; >>> + struct blk_zone *zone; >>> unsigned long flags; >>> + u32 wp_offset; >>> + bool use_write_same = false; >>> >>> zone = blk_lookup_zone(rq->q, sector); >>> - if (!zone) >>> + if (!zone) { >>> + /* Test for a runt zone before giving up */ >>> + if (sdp->type != TYPE_ZBC) { >>> + struct request_queue *q = rq->q; >>> + struct rb_node *node; >>> + >>> + node = rb_last(&q->zones); >>> + if (node) >>> + zone = rb_entry(node, struct blk_zone, node); >>> + if (zone) { >>> + spin_lock_irqsave(&zone->lock, flags); >>> + if ((zone->start + zone->len) <= sector) >>> + goto out; >>> + spin_unlock_irqrestore(&zone->lock, flags); >>> + zone = NULL; >>> + } >>> + } >>> return BLKPREP_KILL; >>> + } >> >> I do not understand the point of this code here to test for the runt >> zone. As long as sector is within the device maximum capacity (in 512B >> unit), blk_lookup_zone will return the pointer to the zone structure >> containing that sector (the RB-tree does not have any constraint >> regarding zone size). The only case where NULL would be returned is if >> discard is issued super early right after the disk is probed and before >> the zone refresh work has completed. We can certainly protect against >> that by delaying the discard. > > As you can see I am not including Host Managed in the > runt check. Indeed, but having a runt zone could also happen with a host-managed disk. > Also you may note that in my patch to get Host Aware working > with the zone cache I do not include the runt zone in the cache. Why not ? The RB-tree will handle it just fine (the insert and lookup code as Hannes had them was not relying on a constant zone size). > So as it sits I need this fallback otherwise doing blkdiscard over > the whole device ends in a error, as well as mkfs.f2fs et. al. Got it, but I do not see a problem with including it. I have not checked the code, but the split of a big discard call into "chunks" should be already handling the last chunk and make sure that the operation does not exceed the device capacity (in any case, that's easy to fix in the sd_zbc_setup_discard code). >>> zone->start, zone->state); >>> ret = BLKPREP_DEFER; >>> goto out; >>> @@ -406,46 +428,80 @@ int sd_zbc_setup_discard(struct scsi_disk *sdkp, struct request *rq, >>> if (zone->state == BLK_ZONE_OFFLINE) { >>> /* let the drive fail the command */ >>> sd_zbc_debug_ratelimit(sdkp, >>> - "Discarding offline zone %zu\n", >>> + "Discarding offline zone %zx\n", >>> zone->start); >>> goto out; >>> } >>> - >>> - if (!blk_zone_is_smr(zone)) { >>> + if (blk_zone_is_cmr(zone)) { >>> + use_write_same = true; >>> sd_zbc_debug_ratelimit(sdkp, >>> - "Discarding %s zone %zu\n", >>> - blk_zone_is_cmr(zone) ? "CMR" : "unknown", >>> + "Discarding CMR zone %zx\n", >>> zone->start); >>> - ret = BLKPREP_DONE; >>> goto out; >>> } >> >> Some 10TB host managed disks out there have 1% conventional zone space, >> that is 100GB of capacity. When issuing a "reset all", doing a write >> same in these zones will take forever... If the user really wants zeroes >> in those zones, let it issue a zeroout. >> >> I think that it would a better choice to simply not report >> discard_zeroes_data as true and do nothing for conventional zones reset. > > I think that would be unfortunate for Host Managed but I think it's > the right choice for Host Aware at this time. So either we base > it on disk type or we have some other config flag added to sysfs. I do not see any difference between host managed and host aware. Both define the same behavior for reset, and both end up in a NOP for conventional zone reset (no data "erasure" required by the standard). For write pointer zones, reading unwritten LBAs returns the initialization pattern, with the exception of host-managed disks with the URSWRZ bit set to 0. But that case is covered in sd.c, so the behavior is consistent across all models. So why forcing data zeroing when the standards do not mandate it ? Best regards. -- Damien Le Moal, Ph.D. Sr. Manager, System Software Group, HGST Research, HGST, a Western Digital brand Damien.LeMoal@hgst.com (+81) 0466-98-3593 (ext. 513593) 1 kirihara-cho, Fujisawa, Kanagawa, 252-0888 Japan www.hgst.com