mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: viro@parcelfarce.linux.theplanet.co.uk
To: "Christoph Hellwig" <hch@infradead.org>,
	"Pásztor Szilárd" <silicon@inf.bme.hu>,
	linux-kernel@vger.kernel.org, linux-net@vger.kernel.org,
	"Adrian Bunk" <bunk@fs.tum.de>
Subject: Re: [2.5 patch] let COMX depend on PROC_FS
Date: Tue, 10 Jun 2003 15:14:43 +0100	[thread overview]
Message-ID: <20030610141443.GZ6754@parcelfarce.linux.theplanet.co.uk> (raw)
In-Reply-To: <20030610142614.A25666@infradead.org>

On Tue, Jun 10, 2003 at 02:26:14PM +0100, Christoph Hellwig wrote:
> On Tue, Jun 10, 2003 at 01:55:22PM +0200, Pásztor Szilárd wrote:
> > The drivers are used by some hundreds of cards today but we tell users to
> > get the small kernelpatch from www.itc.hu and the patch, among other things,
> > exports proc_get_inode. There was a process to integrate the patch into the
> > mainstream kernel last year but, due to lack of time on my part, it was
> > suspended. I hope to be able to pick the line up again and clean things up.
> 
> So what about fixing it instead?  The usage of proc_get_inode is broken
> and so is the whole profs mess in the comx driver.  If you want to keep
> the API you need to add a ramfs-style filesystem instead of abusing
> procfs.

"broken" is a very polite way to describe that driver.  Starting with the
idea of mkdir in virtual filesystem (procfs or otherwise) creating and
populating a diretory (unmodifiable, BTW) and rmdir - removing it, even
though it's non-empty (and can't be emptied, due to above).

Guys, that's _sick_.  And that's aside of the shitload of races all over
that code (no locking whatsoever).  And kmalloc(..., GFP_KERNEL) with
interrupts disabled.  And shutting the hardware down before unregistering
netdev (yes, you check that it's down; nothing guarantees that it will
stay down while you do ->hw_exit() and friends).  And so on, and so on...

IOW, driver needs a serious rewrite, starting with its API.

      parent reply	other threads:[~2003-06-10 14:01 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2003-06-08 14:40 Adrian Bunk
2003-06-08 16:49 ` Christoph Hellwig
2003-06-08 16:56   ` Adrian Bunk
2003-06-08 16:58     ` Christoph Hellwig
2003-06-10 11:55       ` Pásztor Szilárd
2003-06-10 13:26         ` Christoph Hellwig
2003-06-10 13:51           ` Pásztor Szilárd
2003-06-10 13:57             ` Christoph Hellwig
2003-06-10 14:11               ` Pásztor Szilárd
2003-06-10 14:14           ` viro [this message]

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=20030610141443.GZ6754@parcelfarce.linux.theplanet.co.uk \
    --to=viro@parcelfarce.linux.theplanet.co.uk \
    --cc=bunk@fs.tum.de \
    --cc=hch@infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-net@vger.kernel.org \
    --cc=silicon@inf.bme.hu \
    /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

all inboxes | Powered by JetHome®