Re: mpt3sas fails to allocate budget_map and detects no devices

[Ming Lei <ming.lei@redhat.com>]

On Thu, Jan 06, 2022 at 04:19:03PM +0000, Martin Wilck wrote:
> On Thu, 2022-01-06 at 23:41 +0800, Ming Lei wrote:
> > On Thu, Jan 06, 2022 at 03:22:53PM +0000, Martin Wilck wrote:
> > > > 
> > > > I'd suggest to fix mpt3sas for avoiding this memory waste.
> > > 
> > > Let's wait for Sreekanth's comment on that.
> > > 
> > > mpt3sas is not the only driver using a low value. Qlogic drivers
> > > set
> > > cmd_per_lun=3, for example (with 3, our logic would use shift=6, so
> > > the
> > > issue I observed wouldn't occur - but it would be prone to cache
> > > line
> > > bouncing).
> > 
> > But qlogic has smaller .can_queue which looks at most 512, .can_queue
> > is
> > the depth for allocating sbitmap, since each sdev->queue_depth is <=
> > .can_queue.
> 
> I'm seeing here (on an old kernel, admittedly) cmd_per_lun=3 and
> can_queue=2038 for qla2xxx and cmd_per_lun=3 and can_queue=5884 for
> lpfc. Both drivers change the queue depth for devices to 64 in their
> slave_configure() methods.
> 
> Many drivers do this, as it's recommended in scsi_host.h. That's quite
> bad in view of the current bitmap allocation logic - we lay out the
> bitmap assuming the depth used will be cmd_per_lun, but that doesn't
> match the actual depth when the device comes online. For qla2xxx, it
> means that we'd allocate the sbitmap with shift=6 (64 bits per word),
> thus using just a single cache line for 64 requests. Shift=4 (16 bits
> per word) would be the default shift for depth 64.
> 
> Am I misreading the code? Perhaps we should only allocate a preliminary
> sbitmap in scsi_alloc_sdev, and reallocate it after slave_configure()
> has been called, to get the shift right for the driver's default
> settings?

That looks fine to reallocate it after ->slave_configure() returns,
but we need to freeze the request queue for avoiding any in-flight
scsi command. At that time, freeze should be quick enough.


Thanks,
Ming