linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* make flock_lock_file_wait static
@ 2005-01-09 19:42 Arjan van de Ven
  2005-01-09 22:44 ` Trond Myklebust
  0 siblings, 1 reply; 24+ messages in thread
From: Arjan van de Ven @ 2005-01-09 19:42 UTC (permalink / raw)
  To: viro; +Cc: linux-kernel, akpm

Hi,

the patch below makes flock_lock_file_wait static, because it is only used
(once) in fs/locks.c. Making it static allows gcc to generate better code
(partial or entirely inlining it, gcc 3.4 also optimizes the calling
convention for static functions which are guaranteed only local to the file)


Signed-off-by: Arjan van de Ven <arjan@infradead.org>

diff -purN linux-2.6.10/fs/locks.c linux/fs/locks.c
--- linux-2.6.10/fs/locks.c	2005-01-09 14:51:10.101125171 +0100
+++ linux/fs/locks.c	2005-01-09 20:39:44.930216959 +0100
@@ -1451,7 +1451,7 @@ out_unlock:
  *
  * Add a FLOCK style lock to a file.
  */
-int flock_lock_file_wait(struct file *filp, struct file_lock *fl)
+static int flock_lock_file_wait(struct file *filp, struct file_lock *fl)
 {
 	int error;
 	might_sleep();
@@ -1469,8 +1469,6 @@ int flock_lock_file_wait(struct file *fi
 	return error;
 }
 
-EXPORT_SYMBOL(flock_lock_file_wait);
-
 /**
  *	sys_flock: - flock() system call.
  *	@fd: the file descriptor to lock.
diff -purN linux-2.6.10/include/linux/fs.h linux/include/linux/fs.h
--- linux-2.6.10/include/linux/fs.h	2005-01-09 14:51:10.293101584 +0100
+++ linux/include/linux/fs.h	2005-01-09 20:39:18.678389875 +0100
@@ -709,7 +709,6 @@ extern int posix_lock_file_wait(struct f
 extern void posix_block_lock(struct file_lock *, struct file_lock *);
 extern void posix_unblock_lock(struct file *, struct file_lock *);
 extern int posix_locks_deadlock(struct file_lock *, struct file_lock *);
-extern int flock_lock_file_wait(struct file *filp, struct file_lock *fl);
 extern int __break_lease(struct inode *inode, unsigned int flags);
 extern void lease_get_mtime(struct inode *, struct timespec *time);
 extern int setlease(struct file *, long, struct file_lock **);


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: make flock_lock_file_wait static
  2005-01-09 19:42 make flock_lock_file_wait static Arjan van de Ven
@ 2005-01-09 22:44 ` Trond Myklebust
  2005-01-10  8:19   ` Arjan van de Ven
  2005-01-10  8:35   ` Ken Preslan
  0 siblings, 2 replies; 24+ messages in thread
From: Trond Myklebust @ 2005-01-09 22:44 UTC (permalink / raw)
  To: Arjan van de Ven; +Cc: viro, linux-kernel, Andrew Morton

su den 09.01.2005 Klokka 19:42 (+0000) skreiv Arjan van de Ven:
> Hi,
> 
> the patch below makes flock_lock_file_wait static, because it is only used
> (once) in fs/locks.c. Making it static allows gcc to generate better code
> (partial or entirely inlining it, gcc 3.4 also optimizes the calling
> convention for static functions which are guaranteed only local to the file)

Veto. That function is also there for those filesystems that need to
mirror their locks in the VFS. I believe the GFS people are already
using it (they implemented all this anyway), and sooner or later, NFS is
going to have to do it too...

Cheers,
  Trond
-- 
Trond Myklebust <trond.myklebust@fys.uio.no>


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: make flock_lock_file_wait static
  2005-01-09 22:44 ` Trond Myklebust
@ 2005-01-10  8:19   ` Arjan van de Ven
  2005-01-10  8:38     ` Arjan van de Ven
  2005-01-10  8:35   ` Ken Preslan
  1 sibling, 1 reply; 24+ messages in thread
From: Arjan van de Ven @ 2005-01-10  8:19 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: viro, linux-kernel, Andrew Morton

On Sun, 2005-01-09 at 17:44 -0500, Trond Myklebust wrote:
> su den 09.01.2005 Klokka 19:42 (+0000) skreiv Arjan van de Ven:
> > Hi,
> > 
> > the patch below makes flock_lock_file_wait static, because it is only used
> > (once) in fs/locks.c. Making it static allows gcc to generate better code
> > (partial or entirely inlining it, gcc 3.4 also optimizes the calling
> > convention for static functions which are guaranteed only local to the file)
> 
> Veto. That function is also there for those filesystems that need to
> mirror their locks in the VFS. I believe the GFS people are already
> using it (they implemented all this anyway), and sooner or later, NFS is
> going to have to do it too...

before
# size fs/locks.o
   text    data     bss     dec     hex filename
  14712      48       4   14764    39ac fs/locks.o

after (with static inline)
# size fs/locks.o
   text    data     bss     dec     hex filename
  14648      48       4   14700    396c fs/locks.o


is "sooner or later" and "maybe someone else uses it" worth making
everyone elses kernel bigger by 500 bytes of code ?



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: make flock_lock_file_wait static
  2005-01-09 22:44 ` Trond Myklebust
  2005-01-10  8:19   ` Arjan van de Ven
@ 2005-01-10  8:35   ` Ken Preslan
  2005-01-10  8:44     ` Arjan van de Ven
  1 sibling, 1 reply; 24+ messages in thread
From: Ken Preslan @ 2005-01-10  8:35 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: Arjan van de Ven, viro, linux-kernel, Andrew Morton

On Sun, Jan 09, 2005 at 05:44:10PM -0500, Trond Myklebust wrote:
> su den 09.01.2005 Klokka 19:42 (+0000) skreiv Arjan van de Ven:
> > Hi,
> > 
> > the patch below makes flock_lock_file_wait static, because it is only used
> > (once) in fs/locks.c. Making it static allows gcc to generate better code
> > (partial or entirely inlining it, gcc 3.4 also optimizes the calling
> > convention for static functions which are guaranteed only local to the file)
> 
> Veto. That function is also there for those filesystems that need to
> mirror their locks in the VFS. I believe the GFS people are already
> using it (they implemented all this anyway), and sooner or later, NFS is
> going to have to do it too...

I second the veto.  GFS does use this interface.

-- 
Ken Preslan <kpreslan@redhat.com>


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: make flock_lock_file_wait static
  2005-01-10  8:19   ` Arjan van de Ven
@ 2005-01-10  8:38     ` Arjan van de Ven
  2005-01-10 14:23       ` Trond Myklebust
  0 siblings, 1 reply; 24+ messages in thread
From: Arjan van de Ven @ 2005-01-10  8:38 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: viro, linux-kernel, Andrew Morton

On Mon, 2005-01-10 at 09:19 +0100, Arjan van de Ven wrote:
> On Sun, 2005-01-09 at 17:44 -0500, Trond Myklebust wrote:
> > su den 09.01.2005 Klokka 19:42 (+0000) skreiv Arjan van de Ven:
> > > Hi,
> > > 
> > > the patch below makes flock_lock_file_wait static, because it is only used
> > > (once) in fs/locks.c. Making it static allows gcc to generate better code
> > > (partial or entirely inlining it, gcc 3.4 also optimizes the calling
> > > convention for static functions which are guaranteed only local to the file)
> > 
> > Veto. That function is also there for those filesystems that need to
> > mirror their locks in the VFS. I believe the GFS people are already
> > using it (they implemented all this anyway), and sooner or later, NFS is
> > going to have to do it too...
> 
> before
> # size fs/locks.o
>    text    data     bss     dec     hex filename
>   14712      48       4   14764    39ac fs/locks.o
> 
> after (with static inline)
> # size fs/locks.o
>    text    data     bss     dec     hex filename
>   14648      48       4   14700    396c fs/locks.o
> 
> 
> is "sooner or later" and "maybe someone else uses it" worth making
> everyone elses kernel bigger by 500 bytes of code ?

eh 60 not 500; sorry need coffee



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: make flock_lock_file_wait static
  2005-01-10  8:35   ` Ken Preslan
@ 2005-01-10  8:44     ` Arjan van de Ven
  0 siblings, 0 replies; 24+ messages in thread
From: Arjan van de Ven @ 2005-01-10  8:44 UTC (permalink / raw)
  To: Ken Preslan; +Cc: Trond Myklebust, viro, linux-kernel, Andrew Morton

On Mon, 2005-01-10 at 02:35 -0600, Ken Preslan wrote:
> On Sun, Jan 09, 2005 at 05:44:10PM -0500, Trond Myklebust wrote:
> > su den 09.01.2005 Klokka 19:42 (+0000) skreiv Arjan van de Ven:
> > > Hi,
> > > 
> > > the patch below makes flock_lock_file_wait static, because it is only used
> > > (once) in fs/locks.c. Making it static allows gcc to generate better code
> > > (partial or entirely inlining it, gcc 3.4 also optimizes the calling
> > > convention for static functions which are guaranteed only local to the file)
> > 
> > Veto. That function is also there for those filesystems that need to
> > mirror their locks in the VFS. I believe the GFS people are already
> > using it (they implemented all this anyway), and sooner or later, NFS is
> > going to have to do it too...
> 
> I second the veto.  GFS does use this interface.
> 

when are you going to propose GFS for inclusion? Is that next week or is
that months out? Is that really worth bloating everyones kernel until
then ?


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: make flock_lock_file_wait static
  2005-01-10  8:38     ` Arjan van de Ven
@ 2005-01-10 14:23       ` Trond Myklebust
  2005-01-11  8:31         ` Arjan van de Ven
  0 siblings, 1 reply; 24+ messages in thread
From: Trond Myklebust @ 2005-01-10 14:23 UTC (permalink / raw)
  To: Arjan van de Ven; +Cc: viro, linux-kernel, Andrew Morton

må den 10.01.2005 Klokka 09:38 (+0100) skreiv Arjan van de Ven:
>  
> > is "sooner or later" and "maybe someone else uses it" worth making
> > everyone elses kernel bigger by 500 bytes of code ?
> 
> eh 60 not 500; sorry need coffee

It's an API that provides *necessary* functionality for those
filesystems that wish to override the standard flock(). It was very
recently introduced by a third party, so we haven't had time to code up
an NFS flock yet.
Removing it now will just mean that we have to reintroduce it in a month
or so when NFS and the other filesystems start to catch up.

Cheers,
  Trond
-- 
Trond Myklebust <trond.myklebust@fys.uio.no>


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: make flock_lock_file_wait static
  2005-01-10 14:23       ` Trond Myklebust
@ 2005-01-11  8:31         ` Arjan van de Ven
  2005-01-11 19:16           ` Trond Myklebust
  0 siblings, 1 reply; 24+ messages in thread
From: Arjan van de Ven @ 2005-01-11  8:31 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: viro, linux-kernel, Andrew Morton

On Mon, 2005-01-10 at 09:23 -0500, Trond Myklebust wrote:
> må den 10.01.2005 Klokka 09:38 (+0100) skreiv Arjan van de Ven:
> >  
> > > is "sooner or later" and "maybe someone else uses it" worth making
> > > everyone elses kernel bigger by 500 bytes of code ?
> > 
> > eh 60 not 500; sorry need coffee
> 
> It's an API that provides *necessary* functionality for those
> filesystems that wish to override the standard flock(). It was very
> recently introduced by a third party, so we haven't had time to code up
> an NFS flock yet.

where "recently" is last september....
bloating the kernel unused since then...

> Removing it now will just mean that we have to reintroduce it in a month
> or so when NFS and the other filesystems start to catch up.

if NFS indeed is going to use this in a month then you're probably
right, bloating all kernels for another few weeks (before 2.6.11 is out
anyway) isn't a big deal. I'm looking forward to nfs using this in this
timeframe...

If it is going to take a LOT longer though I still feel it's wrong to
bloat *everyones* kernel with this stuff.

(you may think "it's only 100 bytes", well, there are 700+ other such
functions, total that makes over at least 70Kb of unswappable, wasted
memory if not more.)


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: make flock_lock_file_wait static
  2005-01-11  8:31         ` Arjan van de Ven
@ 2005-01-11 19:16           ` Trond Myklebust
  2005-01-11 19:36             ` Arjan van de Ven
  2005-01-15 21:35             ` Adrian Bunk
  0 siblings, 2 replies; 24+ messages in thread
From: Trond Myklebust @ 2005-01-11 19:16 UTC (permalink / raw)
  To: Arjan van de Ven; +Cc: viro, linux-kernel, Andrew Morton

ty den 11.01.2005 Klokka 09:31 (+0100) skreiv Arjan van de Ven:
> On Mon, 2005-01-10 at 09:23 -0500, Trond Myklebust wrote:
> > må den 10.01.2005 Klokka 09:38 (+0100) skreiv Arjan van de Ven:
> > >  
> > > > is "sooner or later" and "maybe someone else uses it" worth making
> > > > everyone elses kernel bigger by 500 bytes of code ?
> > > 
> > > eh 60 not 500; sorry need coffee
> > 
> > It's an API that provides *necessary* functionality for those
> > filesystems that wish to override the standard flock(). It was very
> > recently introduced by a third party, so we haven't had time to code up
> > an NFS flock yet.
> 
> where "recently" is last september....
> bloating the kernel unused since then...

Feel free to help out if you think the NFS development effort is
understaffed.

> If it is going to take a LOT longer though I still feel it's wrong to
> bloat *everyones* kernel with this stuff.
> 
> (you may think "it's only 100 bytes", well, there are 700+ other such
> functions, total that makes over at least 70Kb of unswappable, wasted
> memory if not more.)

A list of these 700+ unused exported APIs would be very useful so that
we can deprecate and/or get rid of them.


Concerning this case, though, and to make what I said in the earlier
mails (a lot) more explicit.

If you unexport flock_lock_file_wait(), then you might as well back out
the entire bloody ->flock() changeset instead because keeping the
->flock() VFS override support without the functionality to make
implementation practical (which is what you appear to want to do) is a
waste of more than 70 bytes of memory.

Now please go and figure out what it is you actually want to do here.

Trond
-- 
Trond Myklebust <trond.myklebust@fys.uio.no>


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: make flock_lock_file_wait static
  2005-01-11 19:16           ` Trond Myklebust
@ 2005-01-11 19:36             ` Arjan van de Ven
  2005-01-25 18:58               ` Paul E. McKenney
  2005-01-15 21:35             ` Adrian Bunk
  1 sibling, 1 reply; 24+ messages in thread
From: Arjan van de Ven @ 2005-01-11 19:36 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: viro, linux-kernel, Andrew Morton

On Tue, 2005-01-11 at 14:16 -0500, Trond Myklebust wrote:
> > (you may think "it's only 100 bytes", well, there are 700+ other such
> > functions, total that makes over at least 70Kb of unswappable, wasted
> > memory if not more.)
> 
> A list of these 700+ unused exported APIs would be very useful so that
> we can deprecate and/or get rid of them.

http://people.redhat.com/arjanv/unused

has the list of symbols that are unused on an i386 allmodconfig based on
the -bk tree 2 days ago.


> Concerning this case, though, and to make what I said in the earlier
> mails (a lot) more explicit.
> 
> If you unexport flock_lock_file_wait(), then you might as well back out
> the entire bloody ->flock() changeset instead because keeping the
> ->flock() VFS override support without the functionality to make
> implementation practical (which is what you appear to want to do) is a
> waste of more than 70 bytes of memory.
> 
> Now please go and figure out what it is you actually want to do here.

save space most of all, and reduce bloat that is not used.
Again if you're going to use it soon, fine. If not, as you say, the
entire thing should probably go because it's a bunch of unused code,
additions to data structures and conditional branches.



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: make flock_lock_file_wait static
  2005-01-11 19:16           ` Trond Myklebust
  2005-01-11 19:36             ` Arjan van de Ven
@ 2005-01-15 21:35             ` Adrian Bunk
  2005-01-15 22:07               ` Trond Myklebust
  1 sibling, 1 reply; 24+ messages in thread
From: Adrian Bunk @ 2005-01-15 21:35 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: Arjan van de Ven, viro, linux-kernel, Andrew Morton

On Tue, Jan 11, 2005 at 02:16:44PM -0500, Trond Myklebust wrote:
> ty den 11.01.2005 Klokka 09:31 (+0100) skreiv Arjan van de Ven:
>...
> > If it is going to take a LOT longer though I still feel it's wrong to
> > bloat *everyones* kernel with this stuff.
> > 
> > (you may think "it's only 100 bytes", well, there are 700+ other such
> > functions, total that makes over at least 70Kb of unswappable, wasted
> > memory if not more.)
> 
> A list of these 700+ unused exported APIs would be very useful so that
> we can deprecate and/or get rid of them.
>...

Patches are already sent for each one that is found (the one by Arjan in 
this discusison is one of them).

My figures in [1] show, any kind of deprecation would mean _much_ extra 
work within the current 2.6 development model.

> Trond

cu
Adrian

[1] http://www.ussg.iu.edu/hypermail/linux/kernel/0501.0/2036.html

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: make flock_lock_file_wait static
  2005-01-15 21:35             ` Adrian Bunk
@ 2005-01-15 22:07               ` Trond Myklebust
  0 siblings, 0 replies; 24+ messages in thread
From: Trond Myklebust @ 2005-01-15 22:07 UTC (permalink / raw)
  To: Adrian Bunk; +Cc: Arjan van de Ven, viro, linux-kernel, Andrew Morton

lau den 15.01.2005 Klokka 22:35 (+0100) skreiv Adrian Bunk:

> My figures in [1] show, any kind of deprecation would mean _much_ extra 
> work within the current 2.6 development model.

Whereas removal of necessary APIs would not? Thanks...

Cleanups are important, but so is actual development....

-- 
Trond Myklebust <trond.myklebust@fys.uio.no>


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: make flock_lock_file_wait static
  2005-01-11 19:36             ` Arjan van de Ven
@ 2005-01-25 18:58               ` Paul E. McKenney
  2005-01-26  3:10                 ` Andrew Morton
                                   ` (2 more replies)
  0 siblings, 3 replies; 24+ messages in thread
From: Paul E. McKenney @ 2005-01-25 18:58 UTC (permalink / raw)
  To: Arjan van de Ven; +Cc: Trond Myklebust, viro, linux-kernel, Andrew Morton

On Tue, Jan 11, 2005 at 08:36:22PM +0100, Arjan van de Ven wrote:
> On Tue, 2005-01-11 at 14:16 -0500, Trond Myklebust wrote:
> > > (you may think "it's only 100 bytes", well, there are 700+ other such
> > > functions, total that makes over at least 70Kb of unswappable, wasted
> > > memory if not more.)
> > 
> > A list of these 700+ unused exported APIs would be very useful so that
> > we can deprecate and/or get rid of them.
> 
> http://people.redhat.com/arjanv/unused
>
> has the list of symbols that are unused on an i386 allmodconfig based on
> the -bk tree 2 days ago.

<donning asbestos suit with the tungsten pinstripes...>

SAN Filesystem is an out-of-tree GPL module that uses the following:

o	blk_get_queue(): used to submit I/O requests using the
	make_request_fn().

o	sock_setsockopt(): used to control communication with other
	nodes in the SAN Filesystem.

o	vfs_follow_link(): used to interpret symbolic links, which
	might point outside of SAN Filesystem.

SDD is a binary module that has committed to get itself to GPL on its
first release after December 31, 2005.  It uses:

o	__read_lock_failed() and __write_lock_failed(): due to SDD's use
	of read_lock() and write_lock().  So, if the plan is to change
	read_lock() and write_lock() to do something different, never mind!

So, could the exports for the following symbols from the list please be
retained through December 31, 2005?

	blk_get_queue
	sock_setsockopt
	vfs_follow_link
	__read_lock_failed
	__write_lock_failed

							Thanx, Paul

PS.  Yes, there are more than two out-of-tree modules in IBM.  Some were
     not affected by this list.  One is looking carefully at Al Viro's
     propagation-node design to see if it does what they need (looks
     promising thus far).  Still others are having more trouble accepting
     the need to stop being binary modules in the near future, but that
     is their problem, not yours!  ;-)  To be fair, at least one in the
     last group has some legitimate IP issues, which we are working on.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: make flock_lock_file_wait static
  2005-01-25 18:58               ` Paul E. McKenney
@ 2005-01-26  3:10                 ` Andrew Morton
  2005-01-26  9:01                 ` Arjan van de Ven
  2005-01-26  9:43                 ` Christoph Hellwig
  2 siblings, 0 replies; 24+ messages in thread
From: Andrew Morton @ 2005-01-26  3:10 UTC (permalink / raw)
  To: paulmck; +Cc: arjan, trond.myklebust, viro, linux-kernel

"Paul E. McKenney" <paulmck@us.ibm.com> wrote:
>
>  So, could the exports for the following symbols from the list please be
>  retained through December 31, 2005?
> 
>  	blk_get_queue
>  	sock_setsockopt
>  	vfs_follow_link
>  	__read_lock_failed
>  	__write_lock_failed

I don't think there's any plan to unexport any of these, and in most cases
it would be a dopey thing to do anyway.  And if we _were_ to try to remove
any of the above exports we should go through the
feature-removal-schedule.txt process.

So I think we're OK?

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: make flock_lock_file_wait static
  2005-01-25 18:58               ` Paul E. McKenney
  2005-01-26  3:10                 ` Andrew Morton
@ 2005-01-26  9:01                 ` Arjan van de Ven
  2005-01-26 16:07                   ` Paul E. McKenney
  2005-01-26  9:43                 ` Christoph Hellwig
  2 siblings, 1 reply; 24+ messages in thread
From: Arjan van de Ven @ 2005-01-26  9:01 UTC (permalink / raw)
  To: paulmck
  Cc: Arjan van de Ven, Trond Myklebust, viro, linux-kernel, Andrew Morton

On Tue, 2005-01-25 at 10:58 -0800, Paul E. McKenney wrote:
> On Tue, Jan 11, 2005 at 08:36:22PM +0100, Arjan van de Ven wrote:
> > On Tue, 2005-01-11 at 14:16 -0500, Trond Myklebust wrote:
> > > > (you may think "it's only 100 bytes", well, there are 700+ other such
> > > > functions, total that makes over at least 70Kb of unswappable, wasted
> > > > memory if not more.)
> > > 
> > > A list of these 700+ unused exported APIs would be very useful so that
> > > we can deprecate and/or get rid of them.
> > 
> > http://people.redhat.com/arjanv/unused
> >
> > has the list of symbols that are unused on an i386 allmodconfig based on
> > the -bk tree 2 days ago.
> 
> <donning asbestos suit with the tungsten pinstripes...>
> 
> SAN Filesystem is an out-of-tree GPL module that uses the following:

any plans to submit this for inclusion?

> 
> o	blk_get_queue(): used to submit I/O requests using the
> 	make_request_fn().

sounds really like the wrong level, any reason to not use submit_bio /
submit_bh instead? Every piece of code outside the core block layer that
I've seen that tries to do this has been wrong/broken to date.

> 
> o	sock_setsockopt(): used to control communication with other
> 	nodes in the SAN Filesystem.
> 

again this very much looks like a misuse; sock_setsocketopt() gets a
*userspace* pointer as argument. Bad API to use (and if you look at
CIFS, they would also like a real nice internal api instead, but don't
use sock_setsockopt() since it's the wrong api)


> SDD is a binary module that has committed to get itself to GPL on its
> first release after December 31, 2005.  It uses:
> 
> o	__read_lock_failed() and __write_lock_failed(): due to SDD's use
> 	of read_lock() and write_lock().  So, if the plan is to change
> 	read_lock() and write_lock() to do something different, never mind!

those two exports are "internal" following from copying the
implementation of read_lock() into the code before compiling it (by the
preprocessor) and currently of course won't go away unless readlocks
change/go away.

Another question: is the SDD module even available for mainline kernels,
or is it only available for distribution kernels ?


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: make flock_lock_file_wait static
  2005-01-25 18:58               ` Paul E. McKenney
  2005-01-26  3:10                 ` Andrew Morton
  2005-01-26  9:01                 ` Arjan van de Ven
@ 2005-01-26  9:43                 ` Christoph Hellwig
  2005-01-26  9:51                   ` Al Viro
  2 siblings, 1 reply; 24+ messages in thread
From: Christoph Hellwig @ 2005-01-26  9:43 UTC (permalink / raw)
  To: Paul E. McKenney
  Cc: Arjan van de Ven, Trond Myklebust, viro, linux-kernel, Andrew Morton

> o	vfs_follow_link(): used to interpret symbolic links, which
> 	might point outside of SAN Filesystem.

This one is going away very soon, including the whole old-style
->follow_link support - for technical reasons.

Please convert your driver to put the contents of the symlink into
... and implement ->put_link like all intree filesystems already did.

Without that we can't bump the limit on recursive symlinks, a feature
that IBM has been pushing for very hard, btw..


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: make flock_lock_file_wait static
  2005-01-26  9:43                 ` Christoph Hellwig
@ 2005-01-26  9:51                   ` Al Viro
  2005-01-26  9:55                     ` Christoph Hellwig
  0 siblings, 1 reply; 24+ messages in thread
From: Al Viro @ 2005-01-26  9:51 UTC (permalink / raw)
  To: Christoph Hellwig, Paul E. McKenney, Arjan van de Ven,
	Trond Myklebust, viro, linux-kernel, Andrew Morton

On Wed, Jan 26, 2005 at 09:43:08AM +0000, Christoph Hellwig wrote:
> > o	vfs_follow_link(): used to interpret symbolic links, which
> > 	might point outside of SAN Filesystem.
> 
> This one is going away very soon, including the whole old-style
> ->follow_link support - for technical reasons.

Not really.  In some cases it _is_ legitimate.  Trivial example:
/proc/self.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: make flock_lock_file_wait static
  2005-01-26  9:51                   ` Al Viro
@ 2005-01-26  9:55                     ` Christoph Hellwig
  2005-01-26 10:00                       ` Al Viro
  0 siblings, 1 reply; 24+ messages in thread
From: Christoph Hellwig @ 2005-01-26  9:55 UTC (permalink / raw)
  To: Al Viro
  Cc: Christoph Hellwig, Paul E. McKenney, Arjan van de Ven,
	Trond Myklebust, viro, linux-kernel, Andrew Morton

On Wed, Jan 26, 2005 at 09:51:31AM +0000, Al Viro wrote:
> On Wed, Jan 26, 2005 at 09:43:08AM +0000, Christoph Hellwig wrote:
> > > o	vfs_follow_link(): used to interpret symbolic links, which
> > > 	might point outside of SAN Filesystem.
> > 
> > This one is going away very soon, including the whole old-style
> > ->follow_link support - for technical reasons.
> 
> Not really.  In some cases it _is_ legitimate.  Trivial example:
> /proc/self.

Which is an extreme special case an not modular.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: make flock_lock_file_wait static
  2005-01-26  9:55                     ` Christoph Hellwig
@ 2005-01-26 10:00                       ` Al Viro
  0 siblings, 0 replies; 24+ messages in thread
From: Al Viro @ 2005-01-26 10:00 UTC (permalink / raw)
  To: Christoph Hellwig, Paul E. McKenney, Arjan van de Ven,
	Trond Myklebust, viro, linux-kernel, Andrew Morton

On Wed, Jan 26, 2005 at 09:55:04AM +0000, Christoph Hellwig wrote:
> On Wed, Jan 26, 2005 at 09:51:31AM +0000, Al Viro wrote:
> > On Wed, Jan 26, 2005 at 09:43:08AM +0000, Christoph Hellwig wrote:
> > > > o	vfs_follow_link(): used to interpret symbolic links, which
> > > > 	might point outside of SAN Filesystem.
> > > 
> > > This one is going away very soon, including the whole old-style
> > > ->follow_link support - for technical reasons.
> > 
> > Not really.  In some cases it _is_ legitimate.  Trivial example:
> > /proc/self.
> 
> Which is an extreme special case an not modular.

I'm not convinced.  Sure, leaving traversal to caller is preferable.
I don't believe that it should be enforced.  That's why we have a way
to enforce a separate limit on that sort of recursion...

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: make flock_lock_file_wait static
  2005-01-26  9:01                 ` Arjan van de Ven
@ 2005-01-26 16:07                   ` Paul E. McKenney
  2005-01-26 18:59                     ` Arjan van de Ven
  0 siblings, 1 reply; 24+ messages in thread
From: Paul E. McKenney @ 2005-01-26 16:07 UTC (permalink / raw)
  To: Arjan van de Ven
  Cc: Arjan van de Ven, Trond Myklebust, viro, linux-kernel, Andrew Morton

On Wed, Jan 26, 2005 at 10:01:00AM +0100, Arjan van de Ven wrote:
> On Tue, 2005-01-25 at 10:58 -0800, Paul E. McKenney wrote:
> > On Tue, Jan 11, 2005 at 08:36:22PM +0100, Arjan van de Ven wrote:
> > > On Tue, 2005-01-11 at 14:16 -0500, Trond Myklebust wrote:
> > > > > (you may think "it's only 100 bytes", well, there are 700+ other such
> > > > > functions, total that makes over at least 70Kb of unswappable, wasted
> > > > > memory if not more.)
> > > > 
> > > > A list of these 700+ unused exported APIs would be very useful so that
> > > > we can deprecate and/or get rid of them.
> > > 
> > > http://people.redhat.com/arjanv/unused
> > >
> > > has the list of symbols that are unused on an i386 allmodconfig based on
> > > the -bk tree 2 days ago.
> > 
> > <donning asbestos suit with the tungsten pinstripes...>
> > 
> > SAN Filesystem is an out-of-tree GPL module that uses the following:
> 
> any plans to submit this for inclusion?

Sigh!  Given that the core of the SAN Filesystem client is a C++
module that runs under Linux, AIX, Windows, &c, I don't have great
hopes for its being accepted for inclusion.  :-(

> > o	blk_get_queue(): used to submit I/O requests using the
> > 	make_request_fn().
> 
> sounds really like the wrong level, any reason to not use submit_bio /
> submit_bh instead? Every piece of code outside the core block layer that
> I've seen that tries to do this has been wrong/broken to date.

I will have them look into this possibility.

> > o	sock_setsockopt(): used to control communication with other
> > 	nodes in the SAN Filesystem.
> 
> again this very much looks like a misuse; sock_setsocketopt() gets a
> *userspace* pointer as argument. Bad API to use (and if you look at
> CIFS, they would also like a real nice internal api instead, but don't
> use sock_setsockopt() since it's the wrong api)

A better API would indeed be a good thing!  I tried a quick search to
find a discussion, but came up dry.  If you have a pointer or contact,
please let me know.  I am also checking with the CIFS people that I know.

> > SDD is a binary module that has committed to get itself to GPL on its
> > first release after December 31, 2005.  It uses:
> > 
> > o	__read_lock_failed() and __write_lock_failed(): due to SDD's use
> > 	of read_lock() and write_lock().  So, if the plan is to change
> > 	read_lock() and write_lock() to do something different, never mind!
> 
> those two exports are "internal" following from copying the
> implementation of read_lock() into the code before compiling it (by the
> preprocessor) and currently of course won't go away unless readlocks
> change/go away.

OK, sounds good!

> Another question: is the SDD module even available for mainline kernels,
> or is it only available for distribution kernels ?

Distributions only.

							Thanx, Paul

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: make flock_lock_file_wait static
  2005-01-26 16:07                   ` Paul E. McKenney
@ 2005-01-26 18:59                     ` Arjan van de Ven
  2005-01-28 14:14                       ` Paul E. McKenney
  0 siblings, 1 reply; 24+ messages in thread
From: Arjan van de Ven @ 2005-01-26 18:59 UTC (permalink / raw)
  To: paulmck
  Cc: Arjan van de Ven, Trond Myklebust, viro, linux-kernel, Andrew Morton

On Wed, 2005-01-26 at 08:07 -0800, Paul E. McKenney wrote:

> > Another question: is the SDD module even available for mainline kernels,
> > or is it only available for distribution kernels ?
> 
> Distributions only.

don't you think it's a bit weird/offensive to ask for exports in the
mainline kernel.org kernel while all you care about is certain vendor
kernels and never plan to provide it for mainline????



^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: make flock_lock_file_wait static
  2005-01-26 18:59                     ` Arjan van de Ven
@ 2005-01-28 14:14                       ` Paul E. McKenney
  2005-01-28 18:50                         ` Christoph Hellwig
  2005-01-28 19:01                         ` Arjan van de Ven
  0 siblings, 2 replies; 24+ messages in thread
From: Paul E. McKenney @ 2005-01-28 14:14 UTC (permalink / raw)
  To: Arjan van de Ven
  Cc: Arjan van de Ven, Trond Myklebust, viro, linux-kernel, Andrew Morton

On Wed, Jan 26, 2005 at 07:59:42PM +0100, Arjan van de Ven wrote:
> On Wed, 2005-01-26 at 08:07 -0800, Paul E. McKenney wrote:
> 
> > > Another question: is the SDD module even available for mainline kernels,
> > > or is it only available for distribution kernels ?
> > 
> > Distributions only.
> 
> don't you think it's a bit weird/offensive to ask for exports in the
> mainline kernel.org kernel while all you care about is certain vendor
> kernels and never plan to provide it for mainline????

In my experience, the only way to get exports into a major distribution
is to get them into mainline kernel.org.  If you can get Red Hat to
change its stance on this, works for me!

						Thanx, Paul

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: make flock_lock_file_wait static
  2005-01-28 14:14                       ` Paul E. McKenney
@ 2005-01-28 18:50                         ` Christoph Hellwig
  2005-01-28 19:01                         ` Arjan van de Ven
  1 sibling, 0 replies; 24+ messages in thread
From: Christoph Hellwig @ 2005-01-28 18:50 UTC (permalink / raw)
  To: Paul E. McKenney
  Cc: Arjan van de Ven, Arjan van de Ven, Trond Myklebust, viro,
	linux-kernel, Andrew Morton

On Fri, Jan 28, 2005 at 06:14:46AM -0800, Paul E. McKenney wrote:
> In my experience, the only way to get exports into a major distribution
> is to get them into mainline kernel.org.  If you can get Red Hat to
> change its stance on this, works for me!

That's not the point.  You're trying to let us work for you for free
without any gain for us.


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: make flock_lock_file_wait static
  2005-01-28 14:14                       ` Paul E. McKenney
  2005-01-28 18:50                         ` Christoph Hellwig
@ 2005-01-28 19:01                         ` Arjan van de Ven
  1 sibling, 0 replies; 24+ messages in thread
From: Arjan van de Ven @ 2005-01-28 19:01 UTC (permalink / raw)
  To: paulmck
  Cc: Arjan van de Ven, Trond Myklebust, viro, linux-kernel, Andrew Morton

On Fri, 2005-01-28 at 06:14 -0800, Paul E. McKenney wrote:
> On Wed, Jan 26, 2005 at 07:59:42PM +0100, Arjan van de Ven wrote:
> > On Wed, 2005-01-26 at 08:07 -0800, Paul E. McKenney wrote:
> > 
> > > > Another question: is the SDD module even available for mainline kernels,
> > > > or is it only available for distribution kernels ?
> > > 
> > > Distributions only.
> > 
> > don't you think it's a bit weird/offensive to ask for exports in the
> > mainline kernel.org kernel while all you care about is certain vendor
> > kernels and never plan to provide it for mainline????
> 
> In my experience, the only way to get exports into a major distribution
> is to get them into mainline kernel.org.  If you can get Red Hat to
> change its stance on this, works for me!

so a business problem between IBM and Red Hat is reason to bloat
everyones kernel ????

that's sooo wrong.

(and no I cannot change the RH policy) 



^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2005-01-28 19:09 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2005-01-09 19:42 make flock_lock_file_wait static Arjan van de Ven
2005-01-09 22:44 ` Trond Myklebust
2005-01-10  8:19   ` Arjan van de Ven
2005-01-10  8:38     ` Arjan van de Ven
2005-01-10 14:23       ` Trond Myklebust
2005-01-11  8:31         ` Arjan van de Ven
2005-01-11 19:16           ` Trond Myklebust
2005-01-11 19:36             ` Arjan van de Ven
2005-01-25 18:58               ` Paul E. McKenney
2005-01-26  3:10                 ` Andrew Morton
2005-01-26  9:01                 ` Arjan van de Ven
2005-01-26 16:07                   ` Paul E. McKenney
2005-01-26 18:59                     ` Arjan van de Ven
2005-01-28 14:14                       ` Paul E. McKenney
2005-01-28 18:50                         ` Christoph Hellwig
2005-01-28 19:01                         ` Arjan van de Ven
2005-01-26  9:43                 ` Christoph Hellwig
2005-01-26  9:51                   ` Al Viro
2005-01-26  9:55                     ` Christoph Hellwig
2005-01-26 10:00                       ` Al Viro
2005-01-15 21:35             ` Adrian Bunk
2005-01-15 22:07               ` Trond Myklebust
2005-01-10  8:35   ` Ken Preslan
2005-01-10  8:44     ` Arjan van de Ven

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).