Please give advise about my first patch attempt

[Thomas Schmitt]

Hi,

Lukas Bulwahn wrote:
> Just take all maintainers as recipients.
> axboe at kernel.dk linux-kernel at vger.kernel.org jejb at linux.ibm.com
> martin.petersen at oracle.com linux-scsi at vger.kernel.org

Really that loudly ?
Ain't linux-kernel at vger.kernel.org too general ?


> It is probably best to base it on v5.9-rc2

Done. The patch was accepted by git apply. git diff looks good.
(There were indeed changes in cdrom.c and cdrom.h. It's not that dead. :))

It compiles, but does not boot:

  [1.099627] nvme nvme0: failed to set APST feature (-10)
  Gave up waiting for root filesystem device

I booted the 5.8 kernel and was happy to see my SSD well and alive.
Then i reverted my changes, compiled and installed the original 5.9-rc2.
Same boot failure (it would have been astonishing if not).

So i cannot test on 5.9-rc2.
(Had to manually rename vmlinuz-5.9.0-rc2-ts and initrd.img-5.9.0-rc2-ts
 before update-grub was willing to create a .cfg which does not boot it
 by default. Eww ... whenever i leave the trodden path ...)


> check if it cleanly applies on the latest linux-next

I read
  https://www.kernel.org/doc/man-pages/linux-next.html
and did without really knowing why:

  $ git remote add linux-next git://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git
  $ git fetch linux-next
  ...
  * [new branch]                akpm          -> linux-next/akpm
  * [new branch]                akpm-base     -> linux-next/akpm-base
  * [new branch]                master        -> linux-next/master
  * [new branch]                pending-fixes -> linux-next/pending-fixes
  * [new branch]                stable        -> linux-next/stable
  * [new tag]                   next-20200827 -> next-20200827
  $ git checkout master
  ...
  $ git remote update
  ...
  Fetching linux-next
  $ git checkout -B linux-next-20200827-ts-issue-2 next-20200827
  Switched to a new branch 'linux-next-20200827-ts-issue-2'
  $ git apply /home/thomas/v5.9-rc2_issue_2.git.diff
  $

Compilation succeeds and it boots.

  dd if=/dev/sr0 bs=2048 count=1 of=/dev/null
  dd if=/dev/sr1 bs=2048 count=1 of=/dev/null
both succeed starting from open tray with a readable medium in it.

So shall i base my patch on next-20200827 ?

-------------------------------------------------------------------------

> > Please also tell me if my mailer (used with this mail) would cause
> > problems.

> I suggest [...] git send-email ... <patch>

I am not sure whether i can get git send-email to work due to local
network issues. My free software mails go out by a primitive SMTP client
which gets fed with plain text files, usually created by vim.

Thus i ask whether my mails show any properties which would hamper
acceptance of a patch, which i would generate by git format-patch.

(I look at patches in https://marc.info/?l=linux-scsi and will try to
 mimick them in my text file as good as possible.)


> I cannot comment on the technical stuff, but that what the mailing list is
> good for

Shall i mention that i am developer of libburn since 2006, which on Linux
operates optical drives from userspace via ioctl(SG_IO) ?


> maybe you are the
> next maintainer for CDROM drivers when you continue to test CDROM :)

Urm. This risk exists by having 7 more cdrom and sr problems fixed in
my local production kernel 4.19, plus 2 of fs/iso9660.
But my clumsiness with community and git will hopefully prevent that.

In general it is not easy to test CDROM beyond sr, because the non-sr
devices are very outdated and need bus hardware which i don't have
any more since years. But sr drives are capricious enough to be fun.

-------------------------------------------------------------------------

Garrit Franke wrote:
> In a recent Patch of mine, Greg nicely pointed out that commits should
> have a standard format across mailing lists: sha ("Commit Message").
> https://www.mail-archive.com/linux-kernel@vger.kernel.org/msg2235132.html

Applied to my commit message draft. Thanks for pointing out.


Greg KH wrote:
> That is also documented in the ever-growing submitting patches document

In next-20200827 it's line 930 of Doc*/process/submitting-patches.rst .

I now plan to write:

  Commit 210ba1d1724f ("[SCSI] sr: update to follow tray status correctly")
  of january 2008 switched sr_drive_status() away from indirectly using
  ...
  By commit 96bcc722c47d ("[SCSI] sr: report more accurate drive status
  after closing the tray.") of july 2008 it now returns CDS_DRIVE_NOT_READY
  ...


Have a nice day :)

Thomas