[{"id":3676244,"web_url":"http://patchwork.ozlabs.org/comment/3676244/","msgid":"","list_archive_url":null,"date":"2026-04-12T07:38:12","subject":"Re: [PATCH 2/3] ata: libata-scsi: enable multi-LUN support for ATAPI\n devices","submitter":{"id":86188,"url":"http://patchwork.ozlabs.org/api/people/86188/","name":"Damien Le Moal","email":"dlemoal@kernel.org"},"content":"On 4/9/26 23:05, Phil Pemberton wrote:\n> libata has never supported multi-LUN ATAPI devices like the Panasonic\n> and NEC PD/CD combo drives due to three limitations:\n> \n> - shost->max_lun is hardcoded to 1 in ata_scsi_add_hosts(), which\n> stops the SCSI layer from probing any LUN other than 0.\n> \n> - __ata_scsi_find_dev() rejects all commands where scsidev->lun != 0,\n> returning NULL which causes DID_BAD_TARGET.\n> \n> - The SCSI-2 CDB LUN field (byte 1, bits 7:5) is never set. Older\n> multi-LUN ATAPI devices rely on this field to route commands to the\n> correct LUN, as transport-layer LUN addressing (per SPC-3+) is not\n> available over the ATA PACKET interface.\n> \n> To fix all three, this change:\n> \n> - Raises max_lun from 1 to 8 (matching the SCSI host default).\n> Sequential LUN scanning stops at the first non-responding LUN, so\n> single-LUN devices are unaffected.\n\nIf the only case that we can encounter with libata are these special ATAPI\ndevices with 2 LUNs, I would limit the maximum to 2.\n\n> - In __ata_scsi_find_dev(), allow non-zero LUNs for ATAPI devices by\n> routing them to the same ata_device as LUN 0.\n> \n> - In atapi_xlat(), encode the target LUN into CDB byte 1 bits 7:5\n> before passing the command packet to the device.\n> \n> These changes are prerequisites for probing additional LUNs during\n> host scanning, which is done in a subsequent patch.\n> \n> Additionally, fix two related issues exposed by multi-LUN scanning:\n> \n> - ata_scsi_dev_config() previously assigned dev->sdev = sdev for every\n> LUN configured. With multiple LUNs sharing one ata_device, this\n> caused dev->sdev to be overwritten by each non-LUN-0 sdev. Restrict\n> the assignment to LUN 0 so that dev->sdev always tracks the\n> canonical scsi_device for the underlying ATA device.\n\nSpecial casing this does not seem nice at all. Why not simply increasing the\nsdev reference count when it is assigned to a LUN that is not LUN 0 ? And drop\nthat reference when the LUN is torn down ? That will remove any dependency on\nthe order in which LUNs are torn down too.\n\n> - ata_scsi_sdev_destroy() detached the entire ATA device whenever\n> dev->sdev was non-NULL. When a spurious multi-LUN scan result was\n> removed, this incorrectly tore down the underlying device. Detach\n> only when the canonical (LUN 0) sdev is being destroyed.\n\nThis should not happen with the reference count change suggested above.\n\nThis is a lot of changes for one patch. So I also suggest splitting this patch.\n\n> \n> Assisted-by: Claude:claude-opus-4-6\n> Signed-off-by: Phil Pemberton \n> ---\n> drivers/ata/libata-scsi.c | 38 ++++++++++++++++++++++++++++++++++----\n> 1 file changed, 34 insertions(+), 4 deletions(-)\n> \n> diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c\n> index 3b65df914ebb..dc6829e60fb3 100644\n> --- a/drivers/ata/libata-scsi.c\n> +++ b/drivers/ata/libata-scsi.c\n> @@ -25,6 +25,7 @@\n> #include \n> #include \n> #include \n> +#include \n> #include \n> #include \n> #include \n> @@ -1131,7 +1132,14 @@ int ata_scsi_dev_config(struct scsi_device *sdev, struct queue_limits *lim,\n> \tif (dev->flags & ATA_DFLAG_TRUSTED)\n> \t\tsdev->security_supported = 1;\n> \n> -\tdev->sdev = sdev;\n> +\t/*\n> +\t * Only LUN 0 is treated as the canonical scsi_device for the ATA\n> +\t * device. Multi-LUN ATAPI devices share a single ata_device, so\n> +\t * dev->sdev must continue to track LUN 0 even when additional LUNs\n> +\t * are added or removed.\n> +\t */\n> +\tif (sdev->lun == 0)\n> +\t\tdev->sdev = sdev;\n> \treturn 0;\n> }\n> \n> @@ -1220,7 +1228,12 @@ void ata_scsi_sdev_destroy(struct scsi_device *sdev)\n> \n> \tspin_lock_irqsave(ap->lock, flags);\n> \tdev = __ata_scsi_find_dev(ap, sdev);\n> -\tif (dev && dev->sdev) {\n> +\t/*\n> +\t * Only detach when the canonical (LUN 0) scsi_device is going away.\n> +\t * Removing a non-LUN-0 sdev (e.g. a spurious multi-LUN scan result)\n> +\t * must not tear down the underlying ATA device.\n> +\t */\n> +\tif (dev && dev->sdev == sdev) {\n> \t\t/* SCSI device already in CANCEL state, no need to offline it */\n> \t\tdev->sdev = NULL;\n> \t\tdev->flags |= ATA_DFLAG_DETACH;\n> @@ -2950,6 +2963,15 @@ static unsigned int atapi_xlat(struct ata_queued_cmd *qc)\n> \tmemset(qc->cdb, 0, dev->cdb_len);\n> \tmemcpy(qc->cdb, scmd->cmnd, scmd->cmd_len);\n> \n> +\t/*\n> +\t * Encode LUN in CDB byte 1 bits 7:5 for multi-LUN ATAPI devices\n> +\t * that use the SCSI-2 CDB LUN convention (e.g. Panasonic PD/CD\n> +\t * combo drives).\n> +\t */\n> +\tif (scmd->device->lun)\n> +\t\tqc->cdb[1] = (qc->cdb[1] & 0x1f) |\n> +\t\t\t ((scmd->device->lun & 0x7) << 5);\n\nSplitting the line after the \"=\" should look nicer.\n\n> +\n> \tqc->complete_fn = atapi_qc_complete;\n> \n> \tqc->tf.flags |= ATA_TFLAG_ISADDR | ATA_TFLAG_DEVICE;\n> @@ -3062,9 +3084,17 @@ static struct ata_device *__ata_scsi_find_dev(struct ata_port *ap,\n> \n> \t/* skip commands not addressed to targets we simulate */\n> \tif (!sata_pmp_attached(ap)) {\n> -\t\tif (unlikely(scsidev->channel || scsidev->lun))\n> +\t\tif (unlikely(scsidev->channel))\n> \t\t\treturn NULL;\n> \t\tdevno = scsidev->id;\n> +\t\t/* Allow non-zero LUNs for ATAPI devices (e.g. PD/CD combos) */\n> +\t\tif (unlikely(scsidev->lun)) {\n> +\t\t\tstruct ata_device *dev = ata_find_dev(ap, devno);\n> +\n> +\t\t\tif (!dev || dev->class != ATA_DEV_ATAPI)\n> +\t\t\t\treturn NULL;\n> +\t\t\treturn dev;\n> +\t\t}\n\nHmmm... What if the multi-LUN ATAPI device is attached to a port multiplier ?\n\n> \t} else {\n> \t\tif (unlikely(scsidev->id || scsidev->lun))\n> \t\t\treturn NULL;\n> @@ -4620,7 +4650,7 @@ int ata_scsi_add_hosts(struct ata_host *host, const struct scsi_host_template *s\n> \t\tshost->transportt = ata_scsi_transport_template;\n> \t\tshost->unique_id = ap->print_id;\n> \t\tshost->max_id = 16;\n> -\t\tshost->max_lun = 1;\n> +\t\tshost->max_lun = 8;\n> \t\tshost->max_channel = 1;\n> \t\tshost->max_cmd_len = 32;\n>","headers":{"Return-Path":"\n ","X-Original-To":["incoming@patchwork.ozlabs.org","linux-ide@vger.kernel.org"],"Delivered-To":"patchwork-incoming@legolas.ozlabs.org","Authentication-Results":["legolas.ozlabs.org;\n\tdkim=pass (2048-bit key;\n unprotected) header.d=kernel.org header.i=@kernel.org header.a=rsa-sha256\n header.s=k20201202 header.b=gCEOMSy6;\n\tdkim-atps=neutral","legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=2600:3c0a:e001:db::12fc:5321; helo=sea.lore.kernel.org;\n envelope-from=linux-ide+bounces-5474-incoming=patchwork.ozlabs.org@vger.kernel.org;\n receiver=patchwork.ozlabs.org)","smtp.subspace.kernel.org;\n\tdkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org\n header.b=\"gCEOMSy6\"","smtp.subspace.kernel.org;\n arc=none smtp.client-ip=10.30.226.201"],"Received":["from sea.lore.kernel.org (sea.lore.kernel.org\n [IPv6:2600:3c0a:e001:db::12fc:5321])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange x25519 server-signature ECDSA (secp384r1) server-digest SHA384)\n\t(No client certificate requested)\n\tby legolas.ozlabs.org (Postfix) with ESMTPS id 4ftj8d5Wvlz1xtJ\n\tfor ; Sun, 12 Apr 2026 17:38:57 +1000 (AEST)","from smtp.subspace.kernel.org (conduit.subspace.kernel.org\n [100.90.174.1])\n\tby sea.lore.kernel.org (Postfix) with ESMTP id 2B2F13016519\n\tfor ; Sun, 12 Apr 2026 07:38:18 +0000 (UTC)","from localhost.localdomain (localhost.localdomain [127.0.0.1])\n\tby smtp.subspace.kernel.org (Postfix) with ESMTP id BD72E3033E8;\n\tSun, 12 Apr 2026 07:38:16 +0000 (UTC)","from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org\n [10.30.226.201])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby smtp.subspace.kernel.org (Postfix) with ESMTPS id 989A62BEC2B;\n\tSun, 12 Apr 2026 07:38:16 +0000 (UTC)","by smtp.kernel.org (Postfix) with ESMTPSA id 5A588C19424;\n\tSun, 12 Apr 2026 07:38:14 +0000 (UTC)"],"ARC-Seal":"i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;\n\tt=1775979496; cv=none;\n b=X05egRKiINjKJ2lK9NWIwSF8Y0QqIwiBECurfMmyvpIllO4OuR+BlcZIh6ElShDMt1oCnOh/sOALPJwp4R8xrVTPWNO7lQy6aqpW7np1RqhbDC2vMgCVi7gh/C6YHF91Y+TONJkPjm8eUV8ERTb1MyjXtGfXNqQZEqNlrYeyETg=","ARC-Message-Signature":"i=1; a=rsa-sha256; d=subspace.kernel.org;\n\ts=arc-20240116; t=1775979496; c=relaxed/simple;\n\tbh=iwmuy2WqzbumP0E/e9Olfsw1ymNLZs3vSpJoDHgPcpI=;\n\th=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From:\n\t In-Reply-To:Content-Type;\n b=XpJGd8985+M+B/bGo89z4v60+0oH+pHOR1OVRBhqs8Emsd4f+Ve9wKaev0Sqiw1cPyId2EsK59VzUkJH8yTjR2RL9FphY0GANEJtIfSKx2gP4TEEn3BggpRSgHUYN0BQ1l1RG/cqxu2iKV0b2Rg/e3CsDXmAgoZOUlBcyxKHCGc=","ARC-Authentication-Results":"i=1; smtp.subspace.kernel.org;\n dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org\n header.b=gCEOMSy6; arc=none smtp.client-ip=10.30.226.201","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org;\n\ts=k20201202; t=1775979496;\n\tbh=iwmuy2WqzbumP0E/e9Olfsw1ymNLZs3vSpJoDHgPcpI=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=gCEOMSy6EGFsMx5WkKNV9UT0hdAPNJVM5FvDBCmqlbZrgDhCwCC0IajGxDNJzlSEz\n\t TdehB/GboIeFO91fwxewOQHrrzOULxLbeGcBqde33vla7c84MC1q0pFdvgwV7jfbvH\n\t BS9nPS5yNCZnihlJCdMAHqvrnbk7mwm/WOoScP9K6g/D0l37FYjleWeSfVdgStXpGc\n\t 6BRXHxgGa7qb10HdzsX2UJRM49L36VxvggeFjjMeea6LVJzOIsLXFQwGC790N3e60N\n\t nJ+vmOqPaOFVKiMEk/0vc2lxnCpRThXEAwyKGyL1LCzGWTwvG3S3wpa/eqSBa8eUSb\n\t KXpfNP7JE+s4w==","Message-ID":"","Date":"Sun, 12 Apr 2026 09:38:12 +0200","Precedence":"bulk","X-Mailing-List":"linux-ide@vger.kernel.org","List-Id":"","List-Subscribe":"","List-Unsubscribe":"","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 2/3] ata: libata-scsi: enable multi-LUN support for ATAPI\n devices","To":"Phil Pemberton , cassel@kernel.org,\n James.Bottomley@HansenPartnership.com, martin.petersen@oracle.com","Cc":"linux-ide@vger.kernel.org, linux-scsi@vger.kernel.org,\n linux-kernel@vger.kernel.org","References":"<20260409210559.155864-1-philpem@philpem.me.uk>\n <20260409210559.155864-3-philpem@philpem.me.uk>","Content-Language":"en-US","From":"Damien Le Moal ","Organization":"Western Digital Research","In-Reply-To":"<20260409210559.155864-3-philpem@philpem.me.uk>","Content-Type":"text/plain; charset=UTF-8","Content-Transfer-Encoding":"7bit"}},{"id":3676369,"web_url":"http://patchwork.ozlabs.org/comment/3676369/","msgid":"","list_archive_url":null,"date":"2026-04-12T19:40:14","subject":"Re: [PATCH 2/3] ata: libata-scsi: enable multi-LUN support for ATAPI\n devices","submitter":{"id":93108,"url":"http://patchwork.ozlabs.org/api/people/93108/","name":"Phil Pemberton","email":"philpem@philpem.me.uk"},"content":"On 12/04/2026 08:38, Damien Le Moal wrote:\n> On 4/9/26 23:05, Phil Pemberton wrote:\n>> - Raises max_lun from 1 to 8 (matching the SCSI host default).\n>> Sequential LUN scanning stops at the first non-responding LUN, so\n>> single-LUN devices are unaffected.\n> \n> If the only case that we can encounter with libata are these special ATAPI\n> devices with 2 LUNs, I would limit the maximum to 2.\n\nI'm inclined to agree, but there are devices with higher LUN counts: the \nNakamichi CD changers. The MJ-4.4 and MJ-5.16 were available in an ATAPI \nvariant which exposed a LUN for each disc in the changer stack. There's \na Cathode Ray Dude video demonstrating the latter.\n\nI like the idea of the lower LUN cap for compatibility, but I think I'd \nhedge bets by also having a module parameter (e.g. libata.atapi_max_lun) \nto override it. Default 2 seems like a good compromise.\n\nIn any case, the BLIST_FORCELUN gate should limit things to one LUN for \nany device which isn't on the device list.\n\n\n>> - ata_scsi_dev_config() previously assigned dev->sdev = sdev for every\n>> LUN configured. With multiple LUNs sharing one ata_device, this\n>> caused dev->sdev to be overwritten by each non-LUN-0 sdev. Restrict\n>> the assignment to LUN 0 so that dev->sdev always tracks the\n>> canonical scsi_device for the underlying ATA device.\n> \n> Special casing this does not seem nice at all. Why not simply increasing the\n> sdev reference count when it is assigned to a LUN that is not LUN 0 ? And drop\n> that reference when the LUN is torn down ? That will remove any dependency on\n> the order in which LUNs are torn down too.\n\nThe if (sdev->lun == 0) guard isn't about teardown ordering; it's about \nwhich device dev->sdev points at.\n\ndev->sdev is a single pointer, but with multi-LUN ATAPI there are now \nmultiple sdevs sharing one ata_device. Without the guard, each call to \nata_scsi_dev_config() overwrites the pointer, so it ends up tracking the \nlast-configured LUN (likely the highest-numbered one).\n\nThis is really made clear by ata_scsi_sdev_destroy(). It uses\n dev->sdev == sdev\nto decide when to schedule ATA-level detach. If the pointer has been \noverwritten, destroying the higher LUN will tear down the whole device, \nand destroying LUN 0 won't trigger a detach.\n\nRefcounting keeps whichever sdev is stored there alive, but it doesn't \ndecide which one should be stored in the first place. Picking LUN 0 \nkeeps the existing invariant intact for single-LUN devices, and the \nother users of dev->sdev (scsi_remove_device() in ata_port_detach(), \nACPI uevents, zpodd) continue to operate on a stable, well-defined sdev.\n\nHappy to add scsi_device_get() on the LUN-0 sdev when a higher LUN is \nconfigured, and the matching put in sdev_destroy, so LUN 0 can't be \nfreed while higher LUNs still exist. That gives you the ordering \nguarantee on top of the pointer-stability guarantee.\n\n>> - ata_scsi_sdev_destroy() detached the entire ATA device whenever\n>> dev->sdev was non-NULL. When a spurious multi-LUN scan result was\n>> removed, this incorrectly tore down the underlying device. Detach\n>> only when the canonical (LUN 0) sdev is being destroyed.\n> \n> This should not happen with the reference count change suggested above.\n> \n> This is a lot of changes for one patch. So I also suggest splitting this patch.\n\nWill do.\n\nAll other comments acked.","headers":{"Return-Path":"\n ","X-Original-To":["incoming@patchwork.ozlabs.org","linux-ide@vger.kernel.org"],"Delivered-To":"patchwork-incoming@legolas.ozlabs.org","Authentication-Results":["legolas.ozlabs.org;\n\tdkim=pass (1024-bit key;\n unprotected) header.d=philpem.me.uk header.i=@philpem.me.uk\n header.a=rsa-sha256 header.s=mail header.b=dqQCg4kG;\n\tdkim-atps=neutral","legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=172.234.253.10; helo=sea.lore.kernel.org;\n envelope-from=linux-ide+bounces-5482-incoming=patchwork.ozlabs.org@vger.kernel.org;\n receiver=patchwork.ozlabs.org)","smtp.subspace.kernel.org;\n\tdkim=pass (1024-bit key) header.d=philpem.me.uk header.i=@philpem.me.uk\n header.b=\"dqQCg4kG\"","smtp.subspace.kernel.org;\n arc=none smtp.client-ip=178.62.38.78","smtp.subspace.kernel.org;\n dmarc=pass (p=reject dis=none) header.from=philpem.me.uk","smtp.subspace.kernel.org;\n spf=pass smtp.mailfrom=philpem.me.uk"],"Received":["from sea.lore.kernel.org (sea.lore.kernel.org [172.234.253.10])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange x25519 server-signature ECDSA (secp384r1) server-digest SHA384)\n\t(No client certificate requested)\n\tby legolas.ozlabs.org (Postfix) with ESMTPS id 4fv1990M7yz1yCx\n\tfor ; Mon, 13 Apr 2026 05:40:28 +1000 (AEST)","from smtp.subspace.kernel.org (conduit.subspace.kernel.org\n [100.90.174.1])\n\tby sea.lore.kernel.org (Postfix) with ESMTP id 7A41930038D2\n\tfor ; Sun, 12 Apr 2026 19:40:25 +0000 (UTC)","from localhost.localdomain (localhost.localdomain [127.0.0.1])\n\tby smtp.subspace.kernel.org (Postfix) with ESMTP id 23C25361DA8;\n\tSun, 12 Apr 2026 19:40:24 +0000 (UTC)","from nick.sneptech.io (nick.sneptech.io [178.62.38.78])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby smtp.subspace.kernel.org (Postfix) with ESMTPS id 6F92B233704;\n\tSun, 12 Apr 2026 19:40:22 +0000 (UTC)","from wolf.philpem.me.uk (81-187-163-148.ip4.reverse-dns.uk\n [81.187.163.148])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange X25519 server-signature RSA-PSS (2048 bits))\n\t(No client certificate requested)\n\t(Authenticated sender: mailrelay_wolf@philpem.me.uk)\n\tby nick.sneptech.io (Postfix) with ESMTPSA id 4CBD0BE5E1;\n\tSun, 12 Apr 2026 19:40:15 +0000 (UTC)","from [10.0.0.32] (cheetah.homenet.philpem.me.uk [10.0.0.32])\n\tby wolf.philpem.me.uk (Postfix) with ESMTPSA id 0826D5FADC;\n\tSun, 12 Apr 2026 20:40:15 +0100 (BST)"],"ARC-Seal":"i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;\n\tt=1776022824; cv=none;\n b=oPL43pUZTHqsxEc9O7COteVRUizTQjRCU4FjPMAbFS1r4AIQWWBj9W5V4zXcD7NaJBBOqMi7bpz2EXrZJK3NPMBYcWlxuqs0Cy/VX0iuLlieKtOYzT0VcHG++GR9yq0cKLhKQNs/MkcwXb12mtYVqIVHHKRh4fyPwigdN+8uZlc=","ARC-Message-Signature":"i=1; a=rsa-sha256; d=subspace.kernel.org;\n\ts=arc-20240116; t=1776022824; c=relaxed/simple;\n\tbh=IDCjVmbBRFYiqoQOS3yQvnMEln4UpziD65rMTyjw+SU=;\n\th=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From:\n\t In-Reply-To:Content-Type;\n b=fBM657lbaG6f/efrL5GdTHPAXMjD6rqRJpXevsiMYK91qdnJ63Zns9Z7h5bRY92/If4eMl8gO1SUd3DlEviG7sA50bnLiHeb0QLmwvYthZK1dA4LgqYTKzT1EuIhpG7AgFpgHf4Xt6+sjYlWAZhm1MmlwVALCy7sdfyOBML+vsA=","ARC-Authentication-Results":"i=1; smtp.subspace.kernel.org;\n dmarc=pass (p=reject dis=none) header.from=philpem.me.uk;\n spf=pass smtp.mailfrom=philpem.me.uk;\n dkim=pass (1024-bit key) header.d=philpem.me.uk header.i=@philpem.me.uk\n header.b=dqQCg4kG; arc=none smtp.client-ip=178.62.38.78","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=philpem.me.uk;\n\ts=mail; t=1776022815;\n\tbh=IDCjVmbBRFYiqoQOS3yQvnMEln4UpziD65rMTyjw+SU=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=dqQCg4kGX9yU3yd6xb+uKT3e1wBfwiarPzdjCYQ7SogeoQSzzitvSbIKKg/PgcwHF\n\t Y5VKvja7jjOQSRNKBAMqJbjMYyeB7tZORhgzU+VcxVUnvgqdTzhw2EzfMutUAxw9jY\n\t +jgg61AZJ2czYzKBkT1lpaWX67GlqCpVyXVCEctU=","Message-ID":"","Date":"Sun, 12 Apr 2026 20:40:14 +0100","Precedence":"bulk","X-Mailing-List":"linux-ide@vger.kernel.org","List-Id":"","List-Subscribe":"","List-Unsubscribe":"","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 2/3] ata: libata-scsi: enable multi-LUN support for ATAPI\n devices","To":"Damien Le Moal , cassel@kernel.org,\n James.Bottomley@HansenPartnership.com, martin.petersen@oracle.com","Cc":"linux-ide@vger.kernel.org, linux-scsi@vger.kernel.org,\n linux-kernel@vger.kernel.org","References":"<20260409210559.155864-1-philpem@philpem.me.uk>\n <20260409210559.155864-3-philpem@philpem.me.uk>\n ","Content-Language":"en-GB","From":"Phil Pemberton ","In-Reply-To":"","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"7bit"}},{"id":3676469,"web_url":"http://patchwork.ozlabs.org/comment/3676469/","msgid":"<05b81015-24fe-417a-b52f-854cbef60de4@kernel.org>","list_archive_url":null,"date":"2026-04-13T05:43:28","subject":"Re: [PATCH 2/3] ata: libata-scsi: enable multi-LUN support for ATAPI\n devices","submitter":{"id":86188,"url":"http://patchwork.ozlabs.org/api/people/86188/","name":"Damien Le Moal","email":"dlemoal@kernel.org"},"content":"On 2026/04/12 21:40, Phil Pemberton wrote:\n> On 12/04/2026 08:38, Damien Le Moal wrote:\n>> On 4/9/26 23:05, Phil Pemberton wrote:\n>>> - Raises max_lun from 1 to 8 (matching the SCSI host default).\n>>> Sequential LUN scanning stops at the first non-responding LUN, so\n>>> single-LUN devices are unaffected.\n>>\n>> If the only case that we can encounter with libata are these special ATAPI\n>> devices with 2 LUNs, I would limit the maximum to 2.\n> \n> I'm inclined to agree, but there are devices with higher LUN counts: the \n> Nakamichi CD changers. The MJ-4.4 and MJ-5.16 were available in an ATAPI \n> variant which exposed a LUN for each disc in the changer stack. There's \n> a Cathode Ray Dude video demonstrating the latter.\n> \n> I like the idea of the lower LUN cap for compatibility, but I think I'd \n> hedge bets by also having a module parameter (e.g. libata.atapi_max_lun) \n> to override it. Default 2 seems like a good compromise.\n> \n> In any case, the BLIST_FORCELUN gate should limit things to one LUN for \n> any device which isn't on the device list.\n\nIdeally, we want to keep the default 1 LUN value and change it to a higher count\nonly if we probe that we are dealing with an ATAPI device (device class is\nATAPI). Not sure if that is possible. Need to look again at that code.\n\n>>> - ata_scsi_dev_config() previously assigned dev->sdev = sdev for every\n>>> LUN configured. With multiple LUNs sharing one ata_device, this\n>>> caused dev->sdev to be overwritten by each non-LUN-0 sdev. Restrict\n>>> the assignment to LUN 0 so that dev->sdev always tracks the\n>>> canonical scsi_device for the underlying ATA device.\n>>\n>> Special casing this does not seem nice at all. Why not simply increasing the\n>> sdev reference count when it is assigned to a LUN that is not LUN 0 ? And drop\n>> that reference when the LUN is torn down ? That will remove any dependency on\n>> the order in which LUNs are torn down too.\n> \n> The if (sdev->lun == 0) guard isn't about teardown ordering; it's about \n> which device dev->sdev points at.\n> \n> dev->sdev is a single pointer, but with multi-LUN ATAPI there are now \n> multiple sdevs sharing one ata_device. Without the guard, each call to \n> ata_scsi_dev_config() overwrites the pointer, so it ends up tracking the \n> last-configured LUN (likely the highest-numbered one).\n> \n> This is really made clear by ata_scsi_sdev_destroy(). It uses\n> dev->sdev == sdev\n> to decide when to schedule ATA-level detach. If the pointer has been \n> overwritten, destroying the higher LUN will tear down the whole device, \n> and destroying LUN 0 won't trigger a detach.\n> \n> Refcounting keeps whichever sdev is stored there alive, but it doesn't \n> decide which one should be stored in the first place. Picking LUN 0 \n> keeps the existing invariant intact for single-LUN devices, and the \n> other users of dev->sdev (scsi_remove_device() in ata_port_detach(), \n> ACPI uevents, zpodd) continue to operate on a stable, well-defined sdev.\n> \n> Happy to add scsi_device_get() on the LUN-0 sdev when a higher LUN is \n> configured, and the matching put in sdev_destroy, so LUN 0 can't be \n> freed while higher LUNs still exist. That gives you the ordering \n> guarantee on top of the pointer-stability guarantee.\n\nOK. I misunderstood your change. I really need to look again at that code, which\nI have not done in a while.\n\nI think your change may be generally OK, but I am worried that things like\nsuspend/resume may have issues. Have you tested that ?\n\nAnyway, please give us some time to look into this (sorry, but I am super busy\nthese days, so it may take a couple of weeks).","headers":{"Return-Path":"\n ","X-Original-To":["incoming@patchwork.ozlabs.org","linux-ide@vger.kernel.org"],"Delivered-To":"patchwork-incoming@legolas.ozlabs.org","Authentication-Results":["legolas.ozlabs.org;\n\tdkim=pass (2048-bit key;\n unprotected) header.d=kernel.org header.i=@kernel.org header.a=rsa-sha256\n header.s=k20201202 header.b=bW51+xBJ;\n\tdkim-atps=neutral","legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=2600:3c04:e001:36c::12fc:5321; helo=tor.lore.kernel.org;\n envelope-from=linux-ide+bounces-5483-incoming=patchwork.ozlabs.org@vger.kernel.org;\n receiver=patchwork.ozlabs.org)","smtp.subspace.kernel.org;\n\tdkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org\n header.b=\"bW51+xBJ\"","smtp.subspace.kernel.org;\n arc=none smtp.client-ip=10.30.226.201"],"Received":["from tor.lore.kernel.org (tor.lore.kernel.org\n [IPv6:2600:3c04:e001:36c::12fc:5321])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange x25519 server-signature ECDSA (secp384r1) server-digest SHA384)\n\t(No client certificate requested)\n\tby legolas.ozlabs.org (Postfix) with ESMTPS id 4fvGY36DqPz1yDF\n\tfor ; Mon, 13 Apr 2026 15:43:35 +1000 (AEST)","from smtp.subspace.kernel.org (conduit.subspace.kernel.org\n [100.90.174.1])\n\tby tor.lore.kernel.org (Postfix) with ESMTP id 9D692300D93E\n\tfor ; Mon, 13 Apr 2026 05:43:32 +0000 (UTC)","from localhost.localdomain (localhost.localdomain [127.0.0.1])\n\tby smtp.subspace.kernel.org (Postfix) with ESMTP id A8F1830E831;\n\tMon, 13 Apr 2026 05:43:31 +0000 (UTC)","from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org\n [10.30.226.201])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby smtp.subspace.kernel.org (Postfix) with ESMTPS id 8256030BF4F;\n\tMon, 13 Apr 2026 05:43:31 +0000 (UTC)","by smtp.kernel.org (Postfix) with ESMTPSA id E43CEC116C6;\n\tMon, 13 Apr 2026 05:43:29 +0000 (UTC)"],"ARC-Seal":"i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;\n\tt=1776059011; cv=none;\n b=SSWs/PXjlUYOJLfkLlBhpekHg52dkI2WSHdE4EDVWMM+HLpHmPFXvSjbGuZ+K2mwLZoaomg9P1PQLtYco8VDPaUhn4UumyKMUCt/AMm4ewxfnGJmbrM1H2QzxJynykYGmwU+eFKF4iwps/fK+PsMYKFNzEsrxvEO0xjPwgZr+LA=","ARC-Message-Signature":"i=1; a=rsa-sha256; d=subspace.kernel.org;\n\ts=arc-20240116; t=1776059011; c=relaxed/simple;\n\tbh=r+015Cgw6pwSJOcQ7qwYWHmJEnAfUe35dDrClraOUXM=;\n\th=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From:\n\t In-Reply-To:Content-Type;\n b=CEj3GGWI3UHf5E76t5ac+mH7ffL3j612xOnzzpmLr2jslQ/quxMSLdEs6H6TfzPWGVc2O6HYi+e62rZw/tLf1p8BXVB5pmXgnuoBNc67HpDmz6l6ZhYeUtXNkjA4vKiNJhV+c9vMeDn6OAXt7rFuCJZiRghY3H7GWEtw7ASn0tQ=","ARC-Authentication-Results":"i=1; smtp.subspace.kernel.org;\n dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org\n header.b=bW51+xBJ; arc=none smtp.client-ip=10.30.226.201","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org;\n\ts=k20201202; t=1776059011;\n\tbh=r+015Cgw6pwSJOcQ7qwYWHmJEnAfUe35dDrClraOUXM=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=bW51+xBJYi5zetWUTby/L8IcG4z3zzIlnBuSu0R8LwDSOYQGgrTJ+jX+/Ls6WoIh3\n\t 1qC6NU51p8WTbcb+pj/8GebVIYvRQaKUHgyvZnGmxFgt5X5Kww/JalhhhW97wEi9e5\n\t mbWMv2fDBRKJiu+qDwe6Y71yiDFWYPPHb5aCeTGsPtLhYJTKYvi+u5g5x0d05oqAUs\n\t g5y/ZneKYexUVeiOKIelYK0zsDN+JtKhWjT+uQUNDpzSFEuAsaM/OSW2idzSnOnnSt\n\t i54O9OCQTUyF7Dx35/yRI3dCtV4VXn9hDtVK6UEISKyGiqlVwFwtK90Dr7da/kjG+t\n\t BH5fl+K54RrKw==","Message-ID":"<05b81015-24fe-417a-b52f-854cbef60de4@kernel.org>","Date":"Mon, 13 Apr 2026 07:43:28 +0200","Precedence":"bulk","X-Mailing-List":"linux-ide@vger.kernel.org","List-Id":"","List-Subscribe":"","List-Unsubscribe":"","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 2/3] ata: libata-scsi: enable multi-LUN support for ATAPI\n devices","To":"Phil Pemberton , cassel@kernel.org,\n James.Bottomley@HansenPartnership.com, martin.petersen@oracle.com","Cc":"linux-ide@vger.kernel.org, linux-scsi@vger.kernel.org,\n linux-kernel@vger.kernel.org","References":"<20260409210559.155864-1-philpem@philpem.me.uk>\n <20260409210559.155864-3-philpem@philpem.me.uk>\n \n ","Content-Language":"en-US","From":"Damien Le Moal ","Organization":"Western Digital Research","In-Reply-To":"","Content-Type":"text/plain; charset=UTF-8","Content-Transfer-Encoding":"7bit"}},{"id":3676554,"web_url":"http://patchwork.ozlabs.org/comment/3676554/","msgid":"","list_archive_url":null,"date":"2026-04-13T08:31:11","subject":"Re: [PATCH 2/3] ata: libata-scsi: enable multi-LUN support for ATAPI\n devices","submitter":{"id":992,"url":"http://patchwork.ozlabs.org/api/people/992/","name":"Hannes Reinecke","email":"hare@suse.de"},"content":"On 4/12/26 21:40, Phil Pemberton wrote:\n> On 12/04/2026 08:38, Damien Le Moal wrote:\n>> On 4/9/26 23:05, Phil Pemberton wrote:\n>>>    - Raises max_lun from 1 to 8 (matching the SCSI host default).\n>>>      Sequential LUN scanning stops at the first non-responding LUN, so\n>>>      single-LUN devices are unaffected.\n>>\n>> If the only case that we can encounter with libata are these special \n>> ATAPI\n>> devices with 2 LUNs, I would limit the maximum to 2.\n> \n> I'm inclined to agree, but there are devices with higher LUN counts: the \n> Nakamichi CD changers. The MJ-4.4 and MJ-5.16 were available in an ATAPI \n> variant which exposed a LUN for each disc in the changer stack. There's \n> a Cathode Ray Dude video demonstrating the latter.\n> \nDon't get too excited. This is ancient technology, with an extremely \nnarrow user-base :-)\n'Were available' is not identical to 'in current use', and I'm somewhat\ndisinclined to add support for technology with no users.\n\n> I like the idea of the lower LUN cap for compatibility, but I think I'd \n> hedge bets by also having a module parameter (e.g. libata.atapi_max_lun) \n> to override it. Default 2 seems like a good compromise.\n> \nHmm. I really would like to restrict max lun to 1 (as it somewhat mimics \nthe master/slave thingie on IDE). But higher than that really is \narbitrary; the next sensible limit would be '7', as this is what SCSI-2\ncould handle. I completely fail to see the motivation to have a limit\nsomewhere in between.\n\n> In any case, the BLIST_FORCELUN gate should limit things to one LUN for \n> any device which isn't on the device list.\n> \n> \n>>>    - ata_scsi_dev_config() previously assigned dev->sdev = sdev for \n>>> every\n>>>      LUN configured.  With multiple LUNs sharing one ata_device, this\n>>>      caused dev->sdev to be overwritten by each non-LUN-0 sdev. \n>>> Restrict\n>>>      the assignment to LUN 0 so that dev->sdev always tracks the\n>>>      canonical scsi_device for the underlying ATA device.\n>>\n>> Special casing this does not seem nice at all. Why not simply \n>> increasing the\n>> sdev reference count when it is assigned to a LUN that is not LUN 0 ? \n>> And drop\n>> that reference when the LUN is torn down ? That will remove any \n>> dependency on\n>> the order in which LUNs are torn down too.\n> \n> The if (sdev->lun == 0) guard isn't about teardown ordering; it's about \n> which device dev->sdev points at.\n> \n> dev->sdev is a single pointer, but with multi-LUN ATAPI there are now \n> multiple sdevs sharing one ata_device. Without the guard, each call to \n> ata_scsi_dev_config() overwrites the pointer, so it ends up tracking the \n> last-configured LUN (likely the highest-numbered one).\n> \nAnd how would you address the 'sdev' of LUN 1?\n(and how of LUN 2, if we decide to have one?)\n\nPlease, don't. The correct way would be to convert 'dev->sdev' into\na list.\n\nCheers,\n\nHannes","headers":{"Return-Path":"\n ","X-Original-To":["incoming@patchwork.ozlabs.org","linux-ide@vger.kernel.org"],"Delivered-To":"patchwork-incoming@legolas.ozlabs.org","Authentication-Results":["legolas.ozlabs.org;\n\tdkim=pass (1024-bit key;\n unprotected) header.d=suse.de header.i=@suse.de header.a=rsa-sha256\n header.s=susede2_rsa header.b=RwZLb/8t;\n\tdkim=pass header.d=suse.de header.i=@suse.de header.a=ed25519-sha256\n header.s=susede2_ed25519 header.b=EBODtRp6;\n\tdkim=pass (1024-bit key) header.d=suse.de header.i=@suse.de\n header.a=rsa-sha256 header.s=susede2_rsa header.b=ljDofN1+;\n\tdkim=neutral header.d=suse.de header.i=@suse.de header.a=ed25519-sha256\n header.s=susede2_ed25519 header.b=2SZkS9vg;\n\tdkim-atps=neutral","legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=2600:3c04:e001:36c::12fc:5321; helo=tor.lore.kernel.org;\n envelope-from=linux-ide+bounces-5484-incoming=patchwork.ozlabs.org@vger.kernel.org;\n receiver=patchwork.ozlabs.org)","smtp.subspace.kernel.org;\n\tdkim=pass (1024-bit key) header.d=suse.de header.i=@suse.de\n header.b=\"RwZLb/8t\";\n\tdkim=permerror (0-bit key) header.d=suse.de header.i=@suse.de\n header.b=\"EBODtRp6\";\n\tdkim=pass (1024-bit key) header.d=suse.de header.i=@suse.de\n header.b=\"ljDofN1+\";\n\tdkim=permerror (0-bit key) header.d=suse.de header.i=@suse.de\n header.b=\"2SZkS9vg\"","smtp.subspace.kernel.org;\n arc=none smtp.client-ip=195.135.223.131","smtp.subspace.kernel.org;\n dmarc=pass (p=none dis=none) header.from=suse.de","smtp.subspace.kernel.org;\n spf=pass smtp.mailfrom=suse.de","smtp-out2.suse.de;\n\tdkim=pass header.d=suse.de header.s=susede2_rsa header.b=ljDofN1+;\n\tdkim=pass header.d=suse.de header.s=susede2_ed25519 header.b=2SZkS9vg"],"Received":["from tor.lore.kernel.org (tor.lore.kernel.org\n [IPv6:2600:3c04:e001:36c::12fc:5321])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange x25519)\n\t(No client certificate requested)\n\tby legolas.ozlabs.org (Postfix) with ESMTPS id 4fvLH918Rzz1xtJ\n\tfor ; Mon, 13 Apr 2026 18:31:49 +1000 (AEST)","from smtp.subspace.kernel.org (conduit.subspace.kernel.org\n [100.90.174.1])\n\tby tor.lore.kernel.org (Postfix) with ESMTP id 98E6C3013EF8\n\tfor ; Mon, 13 Apr 2026 08:31:19 +0000 (UTC)","from localhost.localdomain (localhost.localdomain [127.0.0.1])\n\tby smtp.subspace.kernel.org (Postfix) with ESMTP id 4DEA43A9620;\n\tMon, 13 Apr 2026 08:31:16 +0000 (UTC)","from smtp-out2.suse.de (smtp-out2.suse.de [195.135.223.131])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits))\n\t(No client certificate requested)\n\tby smtp.subspace.kernel.org (Postfix) with ESMTPS id B951A3A784B\n\tfor ; Mon, 13 Apr 2026 08:31:14 +0000 (UTC)","from imap1.dmz-prg2.suse.org (imap1.dmz-prg2.suse.org\n [IPv6:2a07:de40:b281:104:10:150:64:97])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest\n SHA256)\n\t(No client certificate requested)\n\tby smtp-out2.suse.de (Postfix) with ESMTPS id DC6FA5BD98;\n\tMon, 13 Apr 2026 08:31:12 +0000 (UTC)","from imap1.dmz-prg2.suse.org (localhost [127.0.0.1])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest\n SHA256)\n\t(No client certificate requested)\n\tby imap1.dmz-prg2.suse.org (Postfix) with ESMTPS id 9279E4ADBC;\n\tMon, 13 Apr 2026 08:31:12 +0000 (UTC)","from dovecot-director2.suse.de ([2a07:de40:b281:106:10:150:64:167])\n\tby imap1.dmz-prg2.suse.org with ESMTPSA\n\tid 24caItCp3GnjEgAAD6G6ig\n\t(envelope-from ); Mon, 13 Apr 2026 08:31:12 +0000"],"ARC-Seal":"i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;\n\tt=1776069076; cv=none;\n b=IsiBESpwPYYJN3y8i88V5XA0hWicJ0l6SEtpqPuF9w0ZbsGEMQ51TGJrXj3RhQ1KJ7JEH4q+Ktxx5Nx9BECWjTiO1PLs9Mdm+aUMbfmgUWBOak2Kwo2pny3cpxbpVlqzQ39B3PPkC7VAP9nZVzZuymLx6Hgflv9LmAzXeh4FV8I=","ARC-Message-Signature":"i=1; a=rsa-sha256; d=subspace.kernel.org;\n\ts=arc-20240116; t=1776069076; c=relaxed/simple;\n\tbh=fdkklMCbojnfNiYtw3kGEIgd1ibUE0TSAFYaewur45s=;\n\th=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From:\n\t In-Reply-To:Content-Type;\n b=QeWw+vUqp0sfe8Y0umw7W8vrXCmSrAkkUTyCdhJfOYl+mgWPQHlhSmqIHaJptvg3hQClca/9kYD5AioPe3/Im3LoOVxO3tZsYYEQ0NIuI69Q6r3USjvqARpsFyKZMeHwXS9yesDtsN6QJJbyaWN4tRFtLsKFSPMR9iN9qYnxoK8=","ARC-Authentication-Results":"i=1; smtp.subspace.kernel.org;\n dmarc=pass (p=none dis=none) header.from=suse.de;\n spf=pass smtp.mailfrom=suse.de;\n dkim=pass (1024-bit key) header.d=suse.de header.i=@suse.de\n header.b=RwZLb/8t;\n dkim=permerror (0-bit key) header.d=suse.de header.i=@suse.de\n header.b=EBODtRp6;\n dkim=pass (1024-bit key) header.d=suse.de header.i=@suse.de\n header.b=ljDofN1+;\n dkim=permerror (0-bit key) header.d=suse.de header.i=@suse.de\n header.b=2SZkS9vg; arc=none smtp.client-ip=195.135.223.131","DKIM-Signature":["v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de;\n s=susede2_rsa;\n\tt=1776069073;\n h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc:\n\t mime-version:mime-version:content-type:content-type:\n\t content-transfer-encoding:content-transfer-encoding:\n\t in-reply-to:in-reply-to:references:references;\n\tbh=sv8pWDWWoOBrXr74WSnQmH+WSrRD4Yzl1Gmj+wFr/QA=;\n\tb=RwZLb/8tPG39inTiQ1+PkFtEoCnmDibXlLUE9EQfP21QiS9RtPzVHl6GXQPj7zlcaUWXbp\n\tk7Rzypl3ZeZpJxMS2agYICCtq6GgCMxIZVx4yWXcSYdP/QEry6FsqgrdptuxM29CuCe5+1\n\tcFvUZybdenmePIQHKg9449ocQ1KZ0uM=","v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de;\n\ts=susede2_ed25519; t=1776069073;\n\th=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc:\n\t mime-version:mime-version:content-type:content-type:\n\t content-transfer-encoding:content-transfer-encoding:\n\t in-reply-to:in-reply-to:references:references;\n\tbh=sv8pWDWWoOBrXr74WSnQmH+WSrRD4Yzl1Gmj+wFr/QA=;\n\tb=EBODtRp62wm3tMg6+KM8NC3MSB4FM0UtR3iRXjNHJUSzM4nX3Y1W0FUc9R6kHfeYs9Koho\n\tTHeHIijoIFNyVFAg==","v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.de;\n s=susede2_rsa;\n\tt=1776069072;\n h=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc:\n\t mime-version:mime-version:content-type:content-type:\n\t content-transfer-encoding:content-transfer-encoding:\n\t in-reply-to:in-reply-to:references:references;\n\tbh=sv8pWDWWoOBrXr74WSnQmH+WSrRD4Yzl1Gmj+wFr/QA=;\n\tb=ljDofN1+Pb2DFUHC6mucqxuDJ9loBSOOP8Hfvln+kZtkiXLoQ5EMbvhSjE11S3AgqgB/Im\n\tbnwPpdQ9DHlPQZ4qRFdRkfF1cu1bEsK+k3D7/Dw4R6kvdpJHQi6Olm+RWgHvJpj1aDLDO0\n\tzKkE31s+ltozxRfTxpciLnrb/UwTeRE=","v=1; a=ed25519-sha256; c=relaxed/relaxed; d=suse.de;\n\ts=susede2_ed25519; t=1776069072;\n\th=from:from:reply-to:date:date:message-id:message-id:to:to:cc:cc:\n\t mime-version:mime-version:content-type:content-type:\n\t content-transfer-encoding:content-transfer-encoding:\n\t in-reply-to:in-reply-to:references:references;\n\tbh=sv8pWDWWoOBrXr74WSnQmH+WSrRD4Yzl1Gmj+wFr/QA=;\n\tb=2SZkS9vgXP/o/o1DS/NIDXxp+nI1uxpgo8SKp6pzTTuKZhZoOFRkNEMKqoi+sMfQjHfMwx\n\tKO9czEu2NRrz/VDw=="],"Message-ID":"","Date":"Mon, 13 Apr 2026 10:31:11 +0200","Precedence":"bulk","X-Mailing-List":"linux-ide@vger.kernel.org","List-Id":"","List-Subscribe":"","List-Unsubscribe":"","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 2/3] ata: libata-scsi: enable multi-LUN support for ATAPI\n devices","To":"Phil Pemberton ,\n Damien Le Moal , cassel@kernel.org,\n James.Bottomley@HansenPartnership.com, martin.petersen@oracle.com","Cc":"linux-ide@vger.kernel.org, linux-scsi@vger.kernel.org,\n linux-kernel@vger.kernel.org","References":"<20260409210559.155864-1-philpem@philpem.me.uk>\n <20260409210559.155864-3-philpem@philpem.me.uk>\n \n ","Content-Language":"en-US","From":"Hannes Reinecke ","In-Reply-To":"","Content-Type":"text/plain; charset=UTF-8; format=flowed","Content-Transfer-Encoding":"8bit","X-Rspamd-Action":"no action","X-Rspamd-Server":"rspamd2.dmz-prg2.suse.org","X-Spamd-Result":"default: False [-4.51 / 50.00];\n\tBAYES_HAM(-3.00)[100.00%];\n\tNEURAL_HAM_LONG(-1.00)[-1.000];\n\tR_DKIM_ALLOW(-0.20)[suse.de:s=susede2_rsa,suse.de:s=susede2_ed25519];\n\tNEURAL_HAM_SHORT(-0.20)[-1.000];\n\tMIME_GOOD(-0.10)[text/plain];\n\tMX_GOOD(-0.01)[];\n\tFUZZY_RATELIMITED(0.00)[rspamd.com];\n\tRCVD_TLS_ALL(0.00)[];\n\tARC_NA(0.00)[];\n\tRBL_SPAMHAUS_BLOCKED_OPENRESOLVER(0.00)[2a07:de40:b281:104:10:150:64:97:from];\n\tRCVD_VIA_SMTP_AUTH(0.00)[];\n\tTO_DN_SOME(0.00)[];\n\tMIME_TRACE(0.00)[0:+];\n\tRECEIVED_SPAMHAUS_BLOCKED_OPENRESOLVER(0.00)[2a07:de40:b281:106:10:150:64:167:received];\n\tMID_RHS_MATCH_FROM(0.00)[];\n\tSPAMHAUS_XBL(0.00)[2a07:de40:b281:104:10:150:64:97:from];\n\tRCVD_COUNT_TWO(0.00)[2];\n\tFROM_EQ_ENVFROM(0.00)[];\n\tFROM_HAS_DN(0.00)[];\n\tRCPT_COUNT_SEVEN(0.00)[8];\n\tDBL_BLOCKED_OPENRESOLVER(0.00)[suse.de:dkim,suse.de:mid,suse.de:email,imap1.dmz-prg2.suse.org:helo,imap1.dmz-prg2.suse.org:rdns];\n\tDNSWL_BLOCKED(0.00)[2a07:de40:b281:106:10:150:64:167:received,2a07:de40:b281:104:10:150:64:97:from];\n\tTO_MATCH_ENVRCPT_ALL(0.00)[];\n\tDKIM_SIGNED(0.00)[suse.de:s=susede2_rsa,suse.de:s=susede2_ed25519];\n\tDKIM_TRACE(0.00)[suse.de:+]","X-Rspamd-Queue-Id":"DC6FA5BD98","X-Spam-Flag":"NO","X-Spam-Score":"-4.51","X-Spam-Level":""}},{"id":3677019,"web_url":"http://patchwork.ozlabs.org/comment/3677019/","msgid":"<3d08e14c-2768-4126-882c-126113aea275@kernel.org>","list_archive_url":null,"date":"2026-04-14T04:02:59","subject":"Re: [PATCH 2/3] ata: libata-scsi: enable multi-LUN support for ATAPI\n devices","submitter":{"id":86188,"url":"http://patchwork.ozlabs.org/api/people/86188/","name":"Damien Le Moal","email":"dlemoal@kernel.org"},"content":"On 2026/04/13 10:31, Hannes Reinecke wrote:\n> On 4/12/26 21:40, Phil Pemberton wrote:\n>> On 12/04/2026 08:38, Damien Le Moal wrote:\n>>> On 4/9/26 23:05, Phil Pemberton wrote:\n>>>>    - Raises max_lun from 1 to 8 (matching the SCSI host default).\n>>>>      Sequential LUN scanning stops at the first non-responding LUN, so\n>>>>      single-LUN devices are unaffected.\n>>>\n>>> If the only case that we can encounter with libata are these special \n>>> ATAPI\n>>> devices with 2 LUNs, I would limit the maximum to 2.\n>>\n>> I'm inclined to agree, but there are devices with higher LUN counts: the \n>> Nakamichi CD changers. The MJ-4.4 and MJ-5.16 were available in an ATAPI \n>> variant which exposed a LUN for each disc in the changer stack. There's \n>> a Cathode Ray Dude video demonstrating the latter.\n>>\n> Don't get too excited. This is ancient technology, with an extremely \n> narrow user-base :-)\n> 'Were available' is not identical to 'in current use', and I'm somewhat\n> disinclined to add support for technology with no users.\n> \n>> I like the idea of the lower LUN cap for compatibility, but I think I'd \n>> hedge bets by also having a module parameter (e.g. libata.atapi_max_lun) \n>> to override it. Default 2 seems like a good compromise.\n>>\n> Hmm. I really would like to restrict max lun to 1 (as it somewhat mimics \n> the master/slave thingie on IDE). But higher than that really is \n> arbitrary; the next sensible limit would be '7', as this is what SCSI-2\n> could handle. I completely fail to see the motivation to have a limit\n> somewhere in between.\n> \n>> In any case, the BLIST_FORCELUN gate should limit things to one LUN for \n>> any device which isn't on the device list.\n>>\n>>\n>>>>    - ata_scsi_dev_config() previously assigned dev->sdev = sdev for \n>>>> every\n>>>>      LUN configured.  With multiple LUNs sharing one ata_device, this\n>>>>      caused dev->sdev to be overwritten by each non-LUN-0 sdev. \n>>>> Restrict\n>>>>      the assignment to LUN 0 so that dev->sdev always tracks the\n>>>>      canonical scsi_device for the underlying ATA device.\n>>>\n>>> Special casing this does not seem nice at all. Why not simply \n>>> increasing the\n>>> sdev reference count when it is assigned to a LUN that is not LUN 0 ? \n>>> And drop\n>>> that reference when the LUN is torn down ? That will remove any \n>>> dependency on\n>>> the order in which LUNs are torn down too.\n>>\n>> The if (sdev->lun == 0) guard isn't about teardown ordering; it's about \n>> which device dev->sdev points at.\n>>\n>> dev->sdev is a single pointer, but with multi-LUN ATAPI there are now \n>> multiple sdevs sharing one ata_device. Without the guard, each call to \n>> ata_scsi_dev_config() overwrites the pointer, so it ends up tracking the \n>> last-configured LUN (likely the highest-numbered one).\n>>\n> And how would you address the 'sdev' of LUN 1?\n> (and how of LUN 2, if we decide to have one?)\n> \n> Please, don't. The correct way would be to convert 'dev->sdev' into\n> a list.\n\nOr an array sized to the max number of LUNs we allow (7 ?). That would be way\neasier to handle than a list I think since we can index that by LUN number.\nUnless LUNs can have any number ? I did not check...\n\nAnd checking suspend/resume will be needed as we likely will need to keep track\nof the number of LUNs already suspended before being able to suspend the port.\nscsi layer should already be doing something like that for the scsi host though\n(host == port here), so luckily, that should not too bad to handle.","headers":{"Return-Path":"\n ","X-Original-To":["incoming@patchwork.ozlabs.org","linux-ide@vger.kernel.org"],"Delivered-To":"patchwork-incoming@legolas.ozlabs.org","Authentication-Results":["legolas.ozlabs.org;\n\tdkim=pass (2048-bit key;\n unprotected) header.d=kernel.org header.i=@kernel.org header.a=rsa-sha256\n header.s=k20201202 header.b=PAPbHGoF;\n\tdkim-atps=neutral","legolas.ozlabs.org;\n spf=pass (sender SPF authorized) smtp.mailfrom=vger.kernel.org\n (client-ip=2600:3c0a:e001:db::12fc:5321; helo=sea.lore.kernel.org;\n envelope-from=linux-ide+bounces-5486-incoming=patchwork.ozlabs.org@vger.kernel.org;\n receiver=patchwork.ozlabs.org)","smtp.subspace.kernel.org;\n\tdkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org\n header.b=\"PAPbHGoF\"","smtp.subspace.kernel.org;\n arc=none smtp.client-ip=10.30.226.201"],"Received":["from sea.lore.kernel.org (sea.lore.kernel.org\n [IPv6:2600:3c0a:e001:db::12fc:5321])\n\t(using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits)\n\t key-exchange x25519 server-signature ECDSA (secp384r1) server-digest SHA384)\n\t(No client certificate requested)\n\tby legolas.ozlabs.org (Postfix) with ESMTPS id 4fvrGg2Bn9z1yDF\n\tfor ; Tue, 14 Apr 2026 14:03:07 +1000 (AEST)","from smtp.subspace.kernel.org (conduit.subspace.kernel.org\n [100.90.174.1])\n\tby sea.lore.kernel.org (Postfix) with ESMTP id 67783302263F\n\tfor ; Tue, 14 Apr 2026 04:03:04 +0000 (UTC)","from localhost.localdomain (localhost.localdomain [127.0.0.1])\n\tby smtp.subspace.kernel.org (Postfix) with ESMTP id AAE462D781E;\n\tTue, 14 Apr 2026 04:03:02 +0000 (UTC)","from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org\n [10.30.226.201])\n\t(using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))\n\t(No client certificate requested)\n\tby smtp.subspace.kernel.org (Postfix) with ESMTPS id 85F322848BE;\n\tTue, 14 Apr 2026 04:03:02 +0000 (UTC)","by smtp.kernel.org (Postfix) with ESMTPSA id AC510C19425;\n\tTue, 14 Apr 2026 04:03:00 +0000 (UTC)"],"ARC-Seal":"i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116;\n\tt=1776139382; cv=none;\n b=PleOd1sHBjFaa3O/jpjY8+d8Fp1Wbn197VMl5nU7fH/SLh4nLoY4fsK3g4F6tSiA5rkrI5Q6tMqhFOydvFwYb/VxYgJQSGuYf++I2mCEusuvFzF6XbEiYAbS2WgpQ9grsafHbOxHDP62s4756YGLbiFPRtRUTVI/5OVbG9sOAAs=","ARC-Message-Signature":"i=1; a=rsa-sha256; d=subspace.kernel.org;\n\ts=arc-20240116; t=1776139382; c=relaxed/simple;\n\tbh=fXLquU1I0wtNzh/pKU9oSrpzoh7lSCfJefNWjtMabHw=;\n\th=Message-ID:Date:MIME-Version:Subject:To:Cc:References:From:\n\t In-Reply-To:Content-Type;\n b=pW3lrpKEe8dqjJ0ta7OlpQUblLtUceS31Sull7qnDV+tAYdfPnQPxgBKAmX/FGoiQ4KJ0SwEXH97SvuDmh8v6jEO+8lovrEoKrQu2JQ9SDEV4+VgQpKEDS8bHYTj5YGLP38IhF8ox+pojuQ/Z3v3qS9MEtltyp+jLymdEvv9FIM=","ARC-Authentication-Results":"i=1; smtp.subspace.kernel.org;\n dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org\n header.b=PAPbHGoF; arc=none smtp.client-ip=10.30.226.201","DKIM-Signature":"v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org;\n\ts=k20201202; t=1776139382;\n\tbh=fXLquU1I0wtNzh/pKU9oSrpzoh7lSCfJefNWjtMabHw=;\n\th=Date:Subject:To:Cc:References:From:In-Reply-To:From;\n\tb=PAPbHGoFLAkomCKVw7crN7RUoNlOj2ZlndE59qYfsDqJgRMiOxsPCQSLTn4G0tOhD\n\t Ull+bAoDRxHYN3LXu9pXrXdq0gJdT1LMs/ZeeANnx+By62W7b1qsdwsOfo8jsQxLS5\n\t 9hzONh6gWxfUAXEHB5PvtPTyyAYHOyK7gahLZvvxo12vyoVA2UVgrAajwm4y7a6G9f\n\t WVXk3+sYicpoVyW/RWtvMzzczbHMLH0JDmqW91XQCzodwxRWDNhcYV2vEZyXDYRDEm\n\t 75s/MqIs6wWc3rRmSasNyZmtIuGwkUguL9IF83Lt9mJJROzre469vHCZDTMPw28AE/\n\t KP/xvNKGm30jQ==","Message-ID":"<3d08e14c-2768-4126-882c-126113aea275@kernel.org>","Date":"Tue, 14 Apr 2026 06:02:59 +0200","Precedence":"bulk","X-Mailing-List":"linux-ide@vger.kernel.org","List-Id":"","List-Subscribe":"","List-Unsubscribe":"","MIME-Version":"1.0","User-Agent":"Mozilla Thunderbird","Subject":"Re: [PATCH 2/3] ata: libata-scsi: enable multi-LUN support for ATAPI\n devices","To":"Hannes Reinecke , Phil Pemberton ,\n cassel@kernel.org, James.Bottomley@HansenPartnership.com,\n martin.petersen@oracle.com","Cc":"linux-ide@vger.kernel.org, linux-scsi@vger.kernel.org,\n linux-kernel@vger.kernel.org","References":"<20260409210559.155864-1-philpem@philpem.me.uk>\n <20260409210559.155864-3-philpem@philpem.me.uk>\n \n \n ","Content-Language":"en-US","From":"Damien Le Moal ","Organization":"Western Digital Research","In-Reply-To":"","Content-Type":"text/plain; charset=UTF-8","Content-Transfer-Encoding":"8bit"}}]