[PATCH 2/3] nvme-pci: remove queue_count_ops for write,poll_queues

[minwoo.im.dev@gmail.com (Minwoo Im)]

On 19-06-10 12:52:19, Minwoo Im wrote:
> 
> > > > > > However, the check is valid, which shouldn't be nop, so could you fix
> > > > > > the check instead of removing it?
> > > > >
> > > > > Hi, Ming.
> > > > >
> > > > > I don't get what you really mean here.   What do you mean "the check is
> > > > > valid"? I don't see any valid checks in queue_count_set(), not just for
> > > > > check for the failure by kstrtoint().  I think current code is just checks
> > > > > the nr_cpus and do nothing after.
> > > > >
> > > > > Instead fixing this check inside of this function, I have posted the next
> > > > > patch in this series to make sure the number of irqs requested not
> > > > > exceed the num_possible_num().
> > > >
> > > > I suggest to cap 'write_queues' or 'poll_writes' to num_possible_num()
> > > > from the beginning, instead of starting with invalid number.
> > >
> > > Ming,
> > >
> > > Thanks for your review :)
> > >
> > > I have already tried to limit the number of queues inside of
> > queue_count_set()
> > > in [1].  But Christoph had suggested to limit the number in
> > nvme_calc_irq_sets()
> > > instead.  It might be my code was not really good at that time, but could
> > you
> > > please have a look at the mail thread below and give some advices for me?
> > >
> > > [1]  http://lists.infradead.org/pipermail/linux-nvme/2019-May/023868.html
> > 
> > Sorry for missing the previous discussion.
> > 
> > IMO, I prefer to the fix in above link. Cause it is the value of
> > kernel_param_ops
> > to make 'write_queue/poll_queue'  correct from the beginning. That is just
> > like OOP's concept in which each method just does one thing. Then we don't
> > need to bother nvme_calc_irq_sets() for verifying/fixing them.
> 
> Christoph,
> 
> What do you think about what Ming's suggested?  I am actually okay with the
> concept that the function just does its own things.
> 
> It would be great if you can give some advices here :)
> 
> Thanks,

(Gentle ping)

Hi Christoph,

Could you please take a look at this patch ?

Thanks,