mbox series

[RFC,0/3] scsi: mpt: Refactor and port to dma_* interface

Message ID 20200903152832.484908-1-alex.dewar90@gmail.com (mailing list archive)
Headers show
Series scsi: mpt: Refactor and port to dma_* interface | expand

Message

Alex Dewar Sept. 3, 2020, 3:28 p.m. UTC 
Hi all,

I started porting over the mpt code from using the old pci_* compat
functions to their dma_* counterparts, but realised that there are many
functions which have a sleepFlags argument, which is actually almost
never actually needed: almost all of the functions are always called
with sleepFlag == CAN_SLEEP anyway. The first patch fixes these cases
and could be applied by itself as a general tidy-up.

The other two patches are functional changes and so I added the RFC tag
just to be extra cautious. Both of these patches involve changing some
allocations from GFP_ATOMIC to GFP_KERNEL so I wanted to make sure that
I wasn't introducing bugs. (Related question: Can you sleep in an ioctl
context....?)

Any feedback would be greatly appreciated!

Best,
Alex

Comments

Martin K. Petersen Sept. 16, 2020, 2:12 a.m. UTC | #1 
Alex,

> Any feedback would be greatly appreciated!

Have you tested your changes?
Alex Dewar Sept. 16, 2020, 4:44 p.m. UTC | #2 
On Tue, Sep 15, 2020 at 10:12:06PM -0400, Martin K. Petersen wrote:
> 
> Alex,
> 
> > Any feedback would be greatly appreciated!
> 
> Have you tested your changes?

No, as I'm afraid I don't have the hardware.

For patch #1 though, I'm not sure that's such an issue, as the
refactoring was really simple, even though the diffstat has ended up
being quite large! I probably should have submitted that one
individually without the RFC tag. Absolutely loads of functions have a
sleepFlag parameter, but I only found one case where this was actually
set to NO_SLEEP. Otherwise, if you follow the call stack it always ends
up being a sleeping case. I verified this by changing functions one at a
time and compile testing. Would you like me to resend this separately? I
feel that this should probably be merged in any case before we discuss
any of the other changes.

If someone who does have the hardware would like to test it though, that'd be
great :-)

Best,
Alex

> 
> -- 
> Martin K. Petersen	Oracle Linux Engineering
Martin K. Petersen Sept. 18, 2020, 3:26 a.m. UTC | #3 
Alex,

>> Have you tested your changes?
>
> No, as I'm afraid I don't have the hardware.

QEMU supports it, I propose you try testing with that.

I hesitate merging big changes to abandoned drivers unless they've been
tested. It's too easy to miss things during review...