public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Greg KH <gregkh@linuxfoundation.org>
To: Yafang Shao <laoar.shao@gmail.com>
Cc: jejb@linux.ibm.com, martin.petersen@oracle.com,
	rafael@kernel.org, linux-kernel@vger.kernel.org,
	linux-scsi@vger.kernel.org
Subject: Re: [PATCH] drivers: base: Introduce a new kernel parameter driver_sync_probe=
Date: Thu, 7 Dec 2023 13:12:35 +0100	[thread overview]
Message-ID: <2023120704-conducive-junkman-2e3f@gregkh> (raw)
In-Reply-To: <CALOAHbAbU8At_iPVCiz9M8=jiGQ_BYFupCbVwsm=d9te85pAyg@mail.gmail.com>

On Thu, Dec 07, 2023 at 07:59:03PM +0800, Yafang Shao wrote:
> On Thu, Dec 7, 2023 at 6:19 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> >
> > On Wed, Dec 06, 2023 at 10:08:40PM +0800, Yafang Shao wrote:
> > > On Wed, Dec 6, 2023 at 9:31 PM Greg KH <gregkh@linuxfoundation.org> wrote:
> > > >
> > > > On Wed, Dec 06, 2023 at 11:53:55AM +0000, Yafang Shao wrote:
> > > > > After upgrading our kernel from version 4.19 to 6.1, certain regressions
> > > > > occurred due to the driver's asynchronous probe behavior. Specifically,
> > > > > the SCSI driver transitioned to an asynchronous probe by default, resulting
> > > > > in a non-fixed root disk behavior. In the prior 4.19 kernel, the root disk
> > > > > was consistently identified as /dev/sda. However, with kernel 6.1, the root
> > > > > disk can be any of /dev/sdX, leading to issues for applications reliant on
> > > > > /dev/sda, notably impacting monitoring systems monitoring the root disk.
> > > >
> > > > Device names are never guaranteed to be stable, ALWAYS use a persistant
> > > > names like a filesystem label or other ways.  Look at /dev/disk/ for the
> > > > needed ways to do this properly.
> > >
> > > The root disk is typically identified as /dev/sda or /dev/vda, right?
> >
> > Depends on your system.  It can also be identified, in the proper way,
> > as /dev/disk/by-uuid/eef0abc1-4039-4c3f-a123-81fc99999993 if you want
> > (note, fake uuid, use your own disk uuid please.)
> >
> > Why not do that?  That's the most stable and recommended way of doing
> > things.
> 
> Adapting to this change isn't straightforward, especially for a large
> fleet of servers. Our monitoring system needs to accommodate and
> adjust accordingly.

Agreed, that can be rough.  But as this is an issue that was caused by a
scsi core change, perhaps the scsi developers can describe why it's ok.

But really, device naming has ALWAYS been known to not be
deterministic, which is why Pat and I did all the driver core work 20+
years ago so that you have the ability to properly name your devices in
a way that is deterministic.  Using the kernel name like sda is NOT
using that functionality, so while it has been nice to see that it has
been stable for you for a while, you are playing with fire here and will
get burned one day when the firmware in your devices decide to change
response times.

> > > While reverting to synchronous probing could ensure
> > > stability, it's worth noting that asynchronous probing can potentially
> > > shorten the reboot duration under specific conditions. Thus, there
> > > might be some resistance to reverting this change as it offers
> > > performance benefits in certain scenarios. That's why I prefer to
> > > introduce a kernel parameter for it.
> >
> > I don't want to add a new parameter that we need to support for forever
> > and add to the complexity of the system unless it is REALLY needed.
> 
> BTW, since there's already a 'driver_async_probe=', introducing
> another 'driver_sync_probe=' wouldn't significantly increase the
> maintenance overhead.

Any new code adds maintenance overhead and complexity, so you have to
justify it's existance especially when you are not going to be the one
maintaining it :)

thanks,

greg k-h

  reply	other threads:[~2023-12-07 12:12 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-12-06 11:53 [PATCH] drivers: base: Introduce a new kernel parameter driver_sync_probe= Yafang Shao
2023-12-06 13:31 ` Greg KH
2023-12-06 14:08   ` Yafang Shao
2023-12-07 10:19     ` Greg KH
2023-12-07 11:59       ` Yafang Shao
2023-12-07 12:12         ` Greg KH [this message]
2023-12-07 12:36           ` Yafang Shao
2023-12-08  5:36             ` Greg KH
2023-12-08  6:49               ` Yafang Shao
2023-12-08  7:15                 ` Greg KH
2023-12-08  7:26                   ` Yafang Shao

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=2023120704-conducive-junkman-2e3f@gregkh \
    --to=gregkh@linuxfoundation.org \
    --cc=jejb@linux.ibm.com \
    --cc=laoar.shao@gmail.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-scsi@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=rafael@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox