linux-modules.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ian Pilcher <arequipeno@gmail.com>
To: Greg KH <greg@kroah.com>
Cc: linux-modules@vger.kernel.org, kernelnewbies@kernelnewbies.org
Subject: Re: Seeking advice on "monkey patching" a driver
Date: Thu, 1 Jul 2021 15:03:12 -0500	[thread overview]
Message-ID: <b6e41740-94b7-e32e-5d57-deb7b730b2d9@gmail.com> (raw)
In-Reply-To: <YN4Ccf96sqMoPJM3@kroah.com>

On 7/1/21 12:59 PM, Greg KH wrote:
> Oh that's horrible, please no, do not do that :)

Indeed it is, but it works, and it meets my main objective, which is to
allow the use of distribution kernel packages and modules.

> How about a third option, the correct one:
> 	- submit your code changes upstream and they get merged into the
> 	  main kernel tree and no monkeypatching is ever needed at all!
> 
> Have you submitted your changes upstream to the existing drivers?  What
> is preventing that from happening today?

There are a couple of reasons that I've never attempted to do this.

* Scope of work - Currently, there is simply no mechanism to call an LED
   trigger from the ahci or libahci modules, presumably because this is
   something that  really ought to be done by the hardware.  So I would
   have to add some sort of generic framework to associate LED triggers
   with AHCI ports.

   I probably also don't really have the knowledge to do this.  I am not
   familiar with locking, memory management, etc. in the kernel.  Just
   because my "hack" works on a specific 2-core NAS doesn't mean that it
   won't cause all sorts of breakage on a higher-performance system with
   more parallelism.

* (Probable) lack of upstream interest - As I mentioned, disk activity
   LEDs really ought to be handled by the hardware.  I don't know of any
   other system that suffers from this particular limitation.  So this
   is a very, very niche use case.  (Most users of this hardware use the
   manufacturer's "firmware".)

   I did ask about this on the linux-ide mailing list long ago when I
   first wrote the modules, but I don't think that I ever received a
   response, which reinforces my belief that upstream isn't likely to be
   receptive.

I've invested significant time in kernel patches in the past, only to
see them ultimately not be accepted, so I would need to know that
upstream was truly interested in such a feature before I would consider
making such a commitment.

-- 
========================================================================
                  In Soviet Russia, Google searches you!
========================================================================

  reply	other threads:[~2021-07-01 20:03 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-01 16:25 Seeking advice on "monkey patching" a driver Ian Pilcher
2021-07-01 17:59 ` Greg KH
2021-07-01 20:03   ` Ian Pilcher [this message]
2021-07-01 21:04     ` jim.cromie
2021-07-02  4:31     ` Greg KH
2021-07-02 12:05       ` Ian Pilcher
2021-07-02 12:20         ` Greg KH
2021-07-02 15:16           ` Ian Pilcher

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b6e41740-94b7-e32e-5d57-deb7b730b2d9@gmail.com \
    --to=arequipeno@gmail.com \
    --cc=greg@kroah.com \
    --cc=kernelnewbies@kernelnewbies.org \
    --cc=linux-modules@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).