Re: [GIT PULL] nvme update for Linux 4.14, take 2

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




Hello Sagi,

Sorry but I doubt that that patch makes it "impossible" to move controller
reset flow to the NVMe core. There are already several function pointers in
the nvme_ctrl_ops data structure and there is one such data structure per
transport. Had you already considered to add a function pointer to that
structure?

I have, that's the trampoline function that I was referring to, it feels
a bit funny to have aa nvme core function that would look like:

int nvme_reinit_request()
{
	return ctrl->ops->reinit_request()
}

I can easily do that, but doesn't it defeat the purpose of blk_mq_ops?

I don't think so. Request reinitialization is an NVMe concept that is
not used by any other block driver, so why should the pointer to the
reinitialization function exist in blk_mq_ops?

The point of blk-mq is to provide all the functionality that drivers
need, even if it is for just a single driver. Functionality that can be
removed from drivers is good. The smaller/simpler we can make the
driver, the better off we are, even if that means adding a bit of
complexity to the core.

Obviously this is a case-by-case decision. For this particular case, I'm
happy with either solution.

If we get to choose, my preference would be to restore the old behavior
because currently existing nvme transports keep their internal ctrl
representation in the tagset->driver_data as they implement
init/exit_request. Bouncing in nvme core would require to change that
and always keep struct nvme_ctrl as the tagset->driver_data and convert
it on every handler...

Everything is doable but it seems like an unneeded hassle to me...



[Index of Archives]     [Linux RAID]     [Linux SCSI]     [Linux ATA RAID]     [IDE]     [Linux Wireless]     [Linux Kernel]     [ATH6KL]     [Linux Bluetooth]     [Linux Netdev]     [Kernel Newbies]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Device Mapper]

  Powered by Linux