From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pl1-f174.google.com (mail-pl1-f174.google.com [209.85.214.174]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 9703F32E15F for ; Thu, 15 Jan 2026 22:34:23 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.214.174 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1768516465; cv=none; b=ifKfhnrg5B8ZnxlekI8huu6n0InZ8bIEowICf9616Vf6OgpgrqwFoI/1pcghNrIoDsX3ESQ4ZWbcRLcFoLHVl/oOBkOI4E9zbhznq/4WBCuD9zxyk5+4vZnCxmnYh+/EFyxyBBJ+5xjRkaow/cSjcL9ZvSg/KES2WbufPEUE2YU= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1768516465; c=relaxed/simple; bh=KjAlPEth4Kr7fEHcdRZ5AgXc4ovOuTwX7cWLc24agZE=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=THAOQf7uaLuMTj3pij89Pc3zfiZjjSE0ABMEuLelq26uYW5BgRDesb0QnvCQWbiZXLX5R1+2B3Our+wjPvfZTIgn6UpxfKhzCoIishNiDXZeTji12V07dJKnx6n11DS5IMeCbIL75pCKcujlGcAdB14TizCxCJ1vE/H7ZS6vuBI= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com; spf=pass smtp.mailfrom=google.com; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b=rkTvRzsu; arc=none smtp.client-ip=209.85.214.174 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=reject dis=none) header.from=google.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=google.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=google.com header.i=@google.com header.b="rkTvRzsu" Received: by mail-pl1-f174.google.com with SMTP id d9443c01a7336-2a35ae38bdfso9415ad.1 for ; Thu, 15 Jan 2026 14:34:23 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=20230601; t=1768516463; x=1769121263; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=TJALzZttJCf8zJO60CKi5L33ydZ1YJ1ewt2BEqKTx8I=; b=rkTvRzsuWSi+3HWX5IVRZbqF0Zp91aaQUsFMPsOREZhmjY3uhWxykfLr8TfBNx8CWT qHZZUeOfJ2RvCsyeDCTrWOAj/kYHZ7OcYckduWfMzlwPSKbJ6xYxYF8tPTt/ar0OMeay nrNYCdSpElGywcZih3YATBGCqcWQ5USxk62DlpvsS3fAPJMzNgYHRswtlC9LzaVqAQ6m RPzc0RwLp7U1msNUVCYBGJT9C+MQpwPUY+6BpUHWmCGaDDb71CZ9BtDpwhUOqssa4Dy+ fC1Y/7PZ+oNRC+lTQdLAWBRzNs8IRdSHsyA7h35tCvwOkwsS8/Hr75fZLn9kXIbsY74n qQtw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1768516463; x=1769121263; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-gg:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=TJALzZttJCf8zJO60CKi5L33ydZ1YJ1ewt2BEqKTx8I=; b=TR9ZuNc8gilFJuO3rn9jggxNT0S5ru2lNHoqopnZSsWSwQokh1e/DdraBNokYWOUzP Edd8JKnCVeT0jnGTuvM1xUuqR2NDGL1YcwzrweHLVj6qpWNpTtZnUyMkcKS+yNCFe6H2 yRhPnDfmJGUjVOhUn/J/oVYitmOfgrebqUkRUp2s0UG0pVs4VxDCtKnebF04zp7PmewJ xiPccuTvvgAi3XaAWTOib1V0W2/zXGs1s7sXY3PoMSBN8Et8IOVsaiGH4C85ssNWZ8Rv XKFuUQccMfjb727tvd4YwqNNgUxHae2Hxibsu2C8wio7wsZlkLlESTCLVEv9pH+2gpqb QDkQ== X-Forwarded-Encrypted: i=1; AJvYcCU2vX7NMaxA1p/TigphynHUiE3cBC+w9zZ+jlq5aUx2D+Sz+2G+s40TsSLf8ZApl4oJlDuKlKJ0sJdeLR4=@vger.kernel.org X-Gm-Message-State: AOJu0Ywmy2NdK6Ik4Zs7mhUGD9K2L89/2mVrou3jar47LkLVU3COGlnd EsQtEJcZnhmG1A+1XxCgJ4t6c7roDrbhiImuM1QjHWJ8jzimKZZC5r0oYS/ZYeC9kw== X-Gm-Gg: AY/fxX4KZCdN9gEYt22F9hdzqDKJCCiSvPzoM2r3eNNz2sXLyDJgVufFxqNFaDIbkvq N4j3usHw7I40Uq0hj52Rgm66UVCiTvWl9FAiTu5MTRKJTmT9mvcU7TCqL/SG6l+acfbfwuYPYv1 15MpFS6MOpZBr5BPn4h/ouY6HmYFDCv3xGputmTUNMQgR/OhuCj6S/320HtvxQMj1Er5ZFrK8p1 JVBJyoyJZRvcqjD8VydTHNP+Ejh/+Hu636P4U/YlNcLB8ZkT7S0JB0oqTQe8feReQykX5fzhzrA Sm/qXlWe5b+2XeVUh/ZviXedL+hq+C/kNaxVOqERnkj8o19TMFa1LKLr0Dy9ozrmNj4Qe4/JP+X Bjq4/IHN+GRO9PSe1pOzSETSzC4V/EyhDxuQJdl9qZS7VZXwiNsJkljBvLPJ1WjawY8jLzs3Sk4 B4csfBbUEMIq7EpiNViASGdnFwaDXNSK8guBoFVejSGA/GmQmqdQ== X-Received: by 2002:a17:902:d2d0:b0:290:8ecf:e9f9 with SMTP id d9443c01a7336-2a71ada6b7dmr366875ad.7.1768516462467; Thu, 15 Jan 2026 14:34:22 -0800 (PST) Received: from google.com (185.29.127.34.bc.googleusercontent.com. [34.127.29.185]) by smtp.gmail.com with ESMTPSA id d9443c01a7336-2a7193dbed7sm2752375ad.63.2026.01.15.14.34.21 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 15 Jan 2026 14:34:22 -0800 (PST) Date: Thu, 15 Jan 2026 14:34:18 -0800 From: Igor Pylypiv To: Bart Van Assche Cc: "James E.J. Bottomley" , "Martin K. Petersen" , linux-scsi@vger.kernel.org, linux-ide@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: [PATCH] scsi: core: Add 'serial' sysfs attribute for SCSI/SATA Message-ID: References: <20260114175115.384741-1-ipylypiv@google.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: On Wed, Jan 14, 2026 at 12:40:16PM -0800, Bart Van Assche wrote: > On 1/14/26 10:51 AM, Igor Pylypiv wrote: > > Add a 'serial' sysfs attribute for SCSI and SATA devices. This attribute > > exposes the Unit Serial Number, which is derived from the Device > > Identification Vital Product Data (VPD) page 0x80. > > > > Whitespace is stripped from the retrieved serial number to handle > > the different alignment (right-aligned for SCSI, potentially > > left-aligned for SATA). As noted in SAT-5 10.5.3, "Although SPC-5 defines > > the PRODUCT SERIAL NUMBER field as right-aligned, ACS-5 does not require > > its SERIAL NUMBER field to be right-aligned. Therefore, right-alignment > > of the PRODUCT SERIAL NUMBER field for the translation is not assured." > > > > This attribute is used by tools such as lsblk to display the serial > > number of block devices. > > How can existing user space tools use a sysfs attribute that has not yet > been implemented? Please explain. The 'serial' sysfs attribute is implemented for NVMe. Since 'serial' sysfs attribute is missing for SCSI/SATA devices, lsblk reports an empty string instead of a serial number. # lsblk --nodeps -o name,serial NAME SERIAL sda sdb > > > +int scsi_vpd_lun_serial(struct scsi_device *sdev, char *sn, size_t sn_size) > > +{ > > + int len; > > + const unsigned char *d; > > + const struct scsi_vpd *vpd_pg80; > > The current convention for declarations in Linux kernel code is to order > these from longest to shortest. In other words, the opposite order of > the above order. > Thanks for pointing this out. I'll reorder the declarations in V2. > > + rcu_read_lock(); > > Please use guard(rcu)() in new code. Thanks. I'll fix this in V2. > > + vpd_pg80 = rcu_dereference(sdev->vpd_pg80); > > + if (!vpd_pg80) { > > + rcu_read_unlock(); > > + return -ENXIO; > > + } > > + > > + len = vpd_pg80->len - 4; > > + d = vpd_pg80->data + 4; > > + > > + /* Skip leading spaces */ > > + while (len > 0 && isspace(*d)) { > > + len--; > > + d++; > > + } > > + > > + /* Skip trailing spaces */ > > + while (len > 0 && isspace(d[len - 1])) > > + len--; > > + > > + if (sn_size < len + 1) { > > + rcu_read_unlock(); > > + return -EINVAL; > > + } > > Has it been considered to call strim() instead of implementing functionality > that is very similar to strim()? Yes, I considered using strim(). strim() modifies the input buffer by replacing first trailing whitespace with '\0' so we can't use it directly on the vpd_pg80->data. The solution would be to copy the whole vpd page data into the sn buffer and call strim() on the sn buffer. strim() returns a pointer to the first non-whitespace character so we would also need to memmove the serial number to the beginning of the sn buffer. All this extra copying seems to be redundant so I went ahead with a simpler solution that does a single memcpy(). > > + return sysfs_emit(buf, "%s\n", buf); > > The C99 standard says that passing the output buffer pointer as an argument > to sprintf()/snprintf() triggers undefined behavior. I'm not sure whether > this also applies to the kernel equivalents of these > functions but it's probably better to be careful. Kernel implementation of sysfs_emit() seems to be fine. The documentation states that sysfs_emit()/sysfs_emit_at() should be used for show() methods. https://github.com/torvalds/linux/blob/603c05a1639f60e0c52c5fdd25cf5e0b44b9bd8e/Documentation/filesystems/sysfs.rst?plain=1#L246-L247 Thanks, Igor > Thanks, > > Bart.