All of lore.kernel.org
 help / color / mirror / Atom feed
From: Greg KH <greg@kroah.com>
To: Joe Thornber <joe@fib011235813.fsnet.co.uk>
Cc: lvm-devel@sistina.com, linux-kernel@vger.kernel.org
Subject: Re: dmfs for 2.5.51
Date: Fri, 13 Dec 2002 09:29:56 -0800	[thread overview]
Message-ID: <20021213172956.GB27800@kroah.com> (raw)
In-Reply-To: <20021213093745.GB1117@reti>

On Fri, Dec 13, 2002 at 09:37:45AM +0000, Joe Thornber wrote:
> Greg,
> 
> On Thu, Dec 12, 2002 at 05:26:19PM -0800, Greg KH wrote:
> > Here's a patch against 2.5.51 with a updated dmfs.
> 
> I've split out your two changes into seperate patches (patches 21 and
> 22) and made them available here:
> 
> http://people.sistina.com/~thornber/patches/2.5-unstable/2.5.51/2.5.51-dmfs-1/

Thanks.

> > with the following modifications:
> > 	- fixed compile time warnings with the dbg() macro (something
> > 	  better should be used here, I just commented it out...)
> 
> I'm not seeing any warnings, which compiler version are you using ?

The latest for Red Hat 7.2: gcc-2.96-112.7.2
Are you using 3.2?

> > 	- changed the dev file to print out the kdev value, not be the
> > 	  actual block device.
> 
> Should we really be exporting a kdev_t to userland, why not just print out
> 
> <major>:<minor>

No, look at the other dev files in sysfs, I stayed consistant with them.

> > With regards to the last change, I didn't follow the way the other files
> > operate with their complex page creation structure, as this is only a
> > simple one line file.  If the lvm developers want me to change this, I
> > will.
> 
> What you've done looks fine to me, though allocating a whole page to
> hold a single number seems overkill.  Why don't you just snprintf into
> a char[] held on the stack ?

Because I forgot you could do copy_to_user() with data on the stack :)
Good point, I'll change it...

> > If not, I would argue that a number of the other files created
> > should be changed to use this simpler format.  Or is there some reason
> > for creating these lists of pages that I'm missing?
> 
> The files can be larger than a single page, which complicates things
> somewhat.

Hm, then using the seq_file interface might be easier.  I'll look into
this.

thanks,

greg k-h

  reply	other threads:[~2002-12-13 17:23 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2002-12-13  1:26 dmfs for 2.5.51 Greg KH
     [not found] ` <3DF93CC9.979CA988@digeo.com>
2002-12-13  5:25   ` Greg KH
2002-12-13  9:58     ` Joe Thornber
2002-12-13 17:32       ` Greg KH
2002-12-13  9:37 ` Joe Thornber
2002-12-13 17:29   ` Greg KH [this message]
2002-12-13 17:43     ` Joe Thornber

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=20021213172956.GB27800@kroah.com \
    --to=greg@kroah.com \
    --cc=joe@fib011235813.fsnet.co.uk \
    --cc=linux-kernel@vger.kernel.org \
    --cc=lvm-devel@sistina.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.