RE: [PATCH 6/7] blk_end_request: remove/unexport end_that_request_*

2007-09-04 Thread Halevy, Benny
Boaz raised my attention to this patchset today...
We suspect we'll still need the extern entry points for handling the bidi 
request in the scsi_io_completion() path as we only want to call
end_that_request_chunk on req->next_rq and never
end_that_request_last.
 
(see 
http://www.bhalevy.com/open-osd/download/linux-2.6.23-rc2_and_iscsi-iscsi-2007_08_09/0005-SCSI-bidi-support.patch)
 
If this is ok with you I'd leave these entry points in place rather than
taking them out and putting them back in later.
 
Benny



From: [EMAIL PROTECTED] on behalf of Kiyoshi Ueda
Sent: Sat 2007-09-01 01:43
To: linux-kernel@vger.kernel.org; [EMAIL PROTECTED]; [EMAIL PROTECTED]; [EMAIL 
PROTECTED]; [EMAIL PROTECTED]; [EMAIL PROTECTED]
Cc: [EMAIL PROTECTED]; [EMAIL PROTECTED]; [EMAIL PROTECTED]
Subject: [PATCH 6/7] blk_end_request: remove/unexport end_that_request_*



This patch removes the following functions:
  o end_that_request_first()
  o end_that_request_chunk()
and stops exporting the functions below:
  o end_that_request_last()

Signed-off-by: Kiyoshi Ueda <[EMAIL PROTECTED]>
Signed-off-by: Jun'ichi Nomura <[EMAIL PROTECTED]>
---
 block/ll_rw_blk.c  |   61 
- include/linux/blkdev.h |   15 

 2 files changed, 21 insertions(+), 55 deletions(-)

diff -rupN 05-ide-cd-change/block/ll_rw_blk.c 
06-remove-old-interface/block/ll_rw_blk.c
--- 05-ide-cd-change/block/ll_rw_blk.c  2007-08-24 12:11:02.0 -0400
+++ 06-remove-old-interface/block/ll_rw_blk.c   2007-08-24 12:19:02.0 
-0400
@@ -3388,6 +3388,20 @@ static void blk_recalc_rq_sectors(struct
}
 }

+/**
+ * __end_that_request_first - end I/O on a request
+ * @req:  the request being processed
+ * @uptodate: 1 for success, 0 for I/O error, < 0 for specific error
+ * @nr_bytes: number of bytes to complete
+ *
+ * Description:
+ * Ends I/O on a number of bytes attached to @req, and sets it up
+ * for the next range of segments (if any) in the cluster.
+ *
+ * Return:
+ * 0 - we are done with this request, call end_that_request_last()
+ * 1 - still buffers pending for this request
+ **/
 static int __end_that_request_first(struct request *req, int uptodate,
int nr_bytes)
 {
@@ -3498,49 +3512,6 @@ static int __end_that_request_first(stru
return 1;
 }

-/**
- * end_that_request_first - end I/O on a request
- * @req:  the request being processed
- * @uptodate: 1 for success, 0 for I/O error, < 0 for specific error
- * @nr_sectors: number of sectors to end I/O on
- *
- * Description:
- * Ends I/O on a number of sectors attached to @req, and sets it up
- * for the next range of segments (if any) in the cluster.
- *
- * Return:
- * 0 - we are done with this request, call end_that_request_last()
- * 1 - still buffers pending for this request
- **/
-int end_that_request_first(struct request *req, int uptodate, int nr_sectors)
-{
-   return __end_that_request_first(req, uptodate, nr_sectors << 9);
-}
-
-EXPORT_SYMBOL(end_that_request_first);
-
-/**
- * end_that_request_chunk - end I/O on a request
- * @req:  the request being processed
- * @uptodate: 1 for success, 0 for I/O error, < 0 for specific error
- * @nr_bytes: number of bytes to complete
- *
- * Description:
- * Ends I/O on a number of bytes attached to @req, and sets it up
- * for the next range of segments (if any). Like end_that_request_first(),
- * but deals with bytes instead of sectors.
- *
- * Return:
- * 0 - we are done with this request, call end_that_request_last()
- * 1 - still buffers pending for this request
- **/
-int end_that_request_chunk(struct request *req, int uptodate, int nr_bytes)
-{
-   return __end_that_request_first(req, uptodate, nr_bytes);
-}
-
-EXPORT_SYMBOL(end_that_request_chunk);
-
 /*
  * splice the completion data to a local structure and hand off to
  * process_completion_queue() to complete the requests
@@ -3620,7 +3591,7 @@ EXPORT_SYMBOL(blk_complete_request);
 /*
  * queue lock must be held
  */
-void end_that_request_last(struct request *req, int uptodate)
+static void end_that_request_last(struct request *req, int uptodate)
 {
struct gendisk *disk = req->rq_disk;
int error;
@@ -3655,8 +3626,6 @@ void end_that_request_last(struct reques
__blk_put_request(req->q, req);
 }

-EXPORT_SYMBOL(end_that_request_last);
-
 void end_request(struct request *req, int uptodate)
 {
__blk_end_request(req, uptodate, sect2byte(req->hard_cur_sectors));
diff -rupN 05-ide-cd-change/include/linux/blkdev.h 
06-remove-old-interface/include/linux/blkdev.h
--- 05-ide-cd-change/include/linux/blkdev.h 2007-08-24 12:21:45.0 
-0400
+++ 06-remove-old-interface/include/linux/blkdev.h  2007-08-24 
12:21:15.0 -0400
@@ -720,19 +720,16 @@ static inline void blk_run_address_space
 }

 /*
- * end_request() and friends. Must be called 

RE: [PATCH 6/7] blk_end_request: remove/unexport end_that_request_*

2007-09-04 Thread Halevy, Benny
Boaz raised my attention to this patchset today...
We suspect we'll still need the extern entry points for handling the bidi 
request in the scsi_io_completion() path as we only want to call
end_that_request_chunk on req-next_rq and never
end_that_request_last.
 
(see 
http://www.bhalevy.com/open-osd/download/linux-2.6.23-rc2_and_iscsi-iscsi-2007_08_09/0005-SCSI-bidi-support.patch)
 
If this is ok with you I'd leave these entry points in place rather than
taking them out and putting them back in later.
 
Benny



From: [EMAIL PROTECTED] on behalf of Kiyoshi Ueda
Sent: Sat 2007-09-01 01:43
To: linux-kernel@vger.kernel.org; [EMAIL PROTECTED]; [EMAIL PROTECTED]; [EMAIL 
PROTECTED]; [EMAIL PROTECTED]; [EMAIL PROTECTED]
Cc: [EMAIL PROTECTED]; [EMAIL PROTECTED]; [EMAIL PROTECTED]
Subject: [PATCH 6/7] blk_end_request: remove/unexport end_that_request_*



This patch removes the following functions:
  o end_that_request_first()
  o end_that_request_chunk()
and stops exporting the functions below:
  o end_that_request_last()

Signed-off-by: Kiyoshi Ueda [EMAIL PROTECTED]
Signed-off-by: Jun'ichi Nomura [EMAIL PROTECTED]
---
 block/ll_rw_blk.c  |   61 
- include/linux/blkdev.h |   15 

 2 files changed, 21 insertions(+), 55 deletions(-)

diff -rupN 05-ide-cd-change/block/ll_rw_blk.c 
06-remove-old-interface/block/ll_rw_blk.c
--- 05-ide-cd-change/block/ll_rw_blk.c  2007-08-24 12:11:02.0 -0400
+++ 06-remove-old-interface/block/ll_rw_blk.c   2007-08-24 12:19:02.0 
-0400
@@ -3388,6 +3388,20 @@ static void blk_recalc_rq_sectors(struct
}
 }

+/**
+ * __end_that_request_first - end I/O on a request
+ * @req:  the request being processed
+ * @uptodate: 1 for success, 0 for I/O error,  0 for specific error
+ * @nr_bytes: number of bytes to complete
+ *
+ * Description:
+ * Ends I/O on a number of bytes attached to @req, and sets it up
+ * for the next range of segments (if any) in the cluster.
+ *
+ * Return:
+ * 0 - we are done with this request, call end_that_request_last()
+ * 1 - still buffers pending for this request
+ **/
 static int __end_that_request_first(struct request *req, int uptodate,
int nr_bytes)
 {
@@ -3498,49 +3512,6 @@ static int __end_that_request_first(stru
return 1;
 }

-/**
- * end_that_request_first - end I/O on a request
- * @req:  the request being processed
- * @uptodate: 1 for success, 0 for I/O error,  0 for specific error
- * @nr_sectors: number of sectors to end I/O on
- *
- * Description:
- * Ends I/O on a number of sectors attached to @req, and sets it up
- * for the next range of segments (if any) in the cluster.
- *
- * Return:
- * 0 - we are done with this request, call end_that_request_last()
- * 1 - still buffers pending for this request
- **/
-int end_that_request_first(struct request *req, int uptodate, int nr_sectors)
-{
-   return __end_that_request_first(req, uptodate, nr_sectors  9);
-}
-
-EXPORT_SYMBOL(end_that_request_first);
-
-/**
- * end_that_request_chunk - end I/O on a request
- * @req:  the request being processed
- * @uptodate: 1 for success, 0 for I/O error,  0 for specific error
- * @nr_bytes: number of bytes to complete
- *
- * Description:
- * Ends I/O on a number of bytes attached to @req, and sets it up
- * for the next range of segments (if any). Like end_that_request_first(),
- * but deals with bytes instead of sectors.
- *
- * Return:
- * 0 - we are done with this request, call end_that_request_last()
- * 1 - still buffers pending for this request
- **/
-int end_that_request_chunk(struct request *req, int uptodate, int nr_bytes)
-{
-   return __end_that_request_first(req, uptodate, nr_bytes);
-}
-
-EXPORT_SYMBOL(end_that_request_chunk);
-
 /*
  * splice the completion data to a local structure and hand off to
  * process_completion_queue() to complete the requests
@@ -3620,7 +3591,7 @@ EXPORT_SYMBOL(blk_complete_request);
 /*
  * queue lock must be held
  */
-void end_that_request_last(struct request *req, int uptodate)
+static void end_that_request_last(struct request *req, int uptodate)
 {
struct gendisk *disk = req-rq_disk;
int error;
@@ -3655,8 +3626,6 @@ void end_that_request_last(struct reques
__blk_put_request(req-q, req);
 }

-EXPORT_SYMBOL(end_that_request_last);
-
 void end_request(struct request *req, int uptodate)
 {
__blk_end_request(req, uptodate, sect2byte(req-hard_cur_sectors));
diff -rupN 05-ide-cd-change/include/linux/blkdev.h 
06-remove-old-interface/include/linux/blkdev.h
--- 05-ide-cd-change/include/linux/blkdev.h 2007-08-24 12:21:45.0 
-0400
+++ 06-remove-old-interface/include/linux/blkdev.h  2007-08-24 
12:21:15.0 -0400
@@ -720,19 +720,16 @@ static inline void blk_run_address_space
 }

 /*
- * end_request() and friends. Must be called with the 

RE: [nfsv4] RE: Finding hardlinks

2007-01-05 Thread Halevy, Benny
> From: [EMAIL PROTECTED] on behalf of Nicolas Williams
> Sent: Fri 1/5/2007 18:40
> To: Halevy, Benny
> Cc: Trond Myklebust; Jan Harkes; Miklos Szeredi; nfsv4@ietf.org; 
> linux-kernel@vger.kernel.org; Mikulas Patocka; linux-fsdevel@vger.kernel.org; 
> Jeff Layton; Arjan van de Ven
> Subject: Re: [nfsv4] RE: Finding hardlinks
> 
> On Thu, Jan 04, 2007 at 12:04:14PM +0200, Benny Halevy wrote:
> > I agree that the way the client implements its cache is out of the protocol
> > scope. But how do you interpret "correct behavior" in section 4.2.1?
> >  "Clients MUST use filehandle comparisons only to improve performance, not 
> > for correct behavior. All clients > need to be prepared for situations in 
> > which it cannot be determined whether two filehandles denote the same > 
> > object and in such cases, avoid making invalid assumptions which might 
> > cause incorrect behavior."
> > Don't you consider data corruption due to cache inconsistency an incorrect 
> > behavior?
> 
> If a file with multiple hardlinks appears to have multiple distinct
> filehandles then a client like Trond's will treat it as multiple
> distinct files (with the same hardlink count, and you won't be able to
> find the other links to them -- oh well).  Can this cause data
> corruption?  Yes, but only if there are applications that rely on the
> different file names referencing the same file, and backup apps on the
> client won't get the hardlinks right either.

Well, this is why the hard links were made, no?
FWIW, I believe that rename of an open file might also produce this problem.


> 
> What I don't understand is why getting the fileid is so hard -- always
> GETATTR when you GETFH and you'll be fine.  I'm guessing that's not as
> difficult as it is to maintain a hash table of fileids.


The problem with NFS is that fileid isn't enough because the client doesn't
know about removes by other clients until it uses the stale filehandle.
Also, quite a few file systems are not keeping fileids unique (this triggered
this thread)
 
> 
> Nico
> --

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [nfsv4] RE: Finding hardlinks

2007-01-05 Thread Halevy, Benny
 From: [EMAIL PROTECTED] on behalf of Nicolas Williams
 Sent: Fri 1/5/2007 18:40
 To: Halevy, Benny
 Cc: Trond Myklebust; Jan Harkes; Miklos Szeredi; nfsv4@ietf.org; 
 linux-kernel@vger.kernel.org; Mikulas Patocka; linux-fsdevel@vger.kernel.org; 
 Jeff Layton; Arjan van de Ven
 Subject: Re: [nfsv4] RE: Finding hardlinks
 
 On Thu, Jan 04, 2007 at 12:04:14PM +0200, Benny Halevy wrote:
  I agree that the way the client implements its cache is out of the protocol
  scope. But how do you interpret correct behavior in section 4.2.1?
   Clients MUST use filehandle comparisons only to improve performance, not 
  for correct behavior. All clients  need to be prepared for situations in 
  which it cannot be determined whether two filehandles denote the same  
  object and in such cases, avoid making invalid assumptions which might 
  cause incorrect behavior.
  Don't you consider data corruption due to cache inconsistency an incorrect 
  behavior?
 
 If a file with multiple hardlinks appears to have multiple distinct
 filehandles then a client like Trond's will treat it as multiple
 distinct files (with the same hardlink count, and you won't be able to
 find the other links to them -- oh well).  Can this cause data
 corruption?  Yes, but only if there are applications that rely on the
 different file names referencing the same file, and backup apps on the
 client won't get the hardlinks right either.

Well, this is why the hard links were made, no?
FWIW, I believe that rename of an open file might also produce this problem.


 
 What I don't understand is why getting the fileid is so hard -- always
 GETATTR when you GETFH and you'll be fine.  I'm guessing that's not as
 difficult as it is to maintain a hash table of fileids.


The problem with NFS is that fileid isn't enough because the client doesn't
know about removes by other clients until it uses the stale filehandle.
Also, quite a few file systems are not keeping fileids unique (this triggered
this thread)
 
 
 Nico
 --

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: [nfsv4] RE: Finding hardlinks

2006-12-31 Thread Halevy, Benny
Trond Myklebust wrote:
>  
> On Thu, 2006-12-28 at 15:07 -0500, Halevy, Benny wrote:
> > Mikulas Patocka wrote:
> 
> > >BTW. how does (or how should?) NFS client deal with cache coherency if 
> > >filehandles for the same file differ?
> > >
> > 
> > Trond can probably answer this better than me...
> > As I read it, currently the nfs client matches both the fileid and the
> > filehandle (in nfs_find_actor). This means that different filehandles
> > for the same file would result in different inodes :(.
> > Strictly following the nfs protocol, comparing only the fileid should
> > be enough IF fileids are indeed unique within the filesystem.
> > Comparing the filehandle works as a workaround when the exported filesystem
> > (or the nfs server) violates that.  From a user stand point I think that
> > this should be configurable, probably per mount point.
> 
> Matching files by fileid instead of filehandle is a lot more trouble
> since fileids may be reused after a file has been deleted. Every time
> you look up a file, and get a new filehandle for the same fileid, you
> would at the very least have to do another GETATTR using one of the
> 'old' filehandles in order to ensure that the file is the same object as
> the one you have cached. Then there is the issue of what to do when you
> open(), read() or write() to the file: which filehandle do you use, are
> the access permissions the same for all filehandles, ...
> 
> All in all, much pain for little or no gain.

See my answer to your previous reply.  It seems like the current
implementation is in violation of the nfs protocol and the extra pain
is required.

> 
> Most servers therefore take great pains to ensure that clients can use
> filehandles to identify inodes. The exceptions tend to be broken in
> other ways

This is true maybe in linux, but not necessarily in non-linux based nfs
servers.

> (Note: knfsd without the no_subtree_check option is one of
> these exceptions - it can break in the case of cross-directory renames).
> 
> Cheers,
>   Trond


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: Finding hardlinks

2006-12-31 Thread Halevy, Benny
Trond Myklebust wrote:
>  
> On Thu, 2006-12-28 at 17:12 +0200, Benny Halevy wrote:
> 
> > As an example, some file systems encode hint information into the filehandle
> > and the hints may change over time, another example is encoding parent
> > information into the filehandle and then handles representing hard links
> > to the same file from different directories will differ.
> 
> Both these examples are bogus. Filehandle information should not change
> over time (except in the special case of NFSv4 "volatile filehandles")
> and they should definitely not encode parent directory information that
> can change over time (think rename()!).
> 
> Cheers
>   Trond
> 

The first one is a real life example.  Hints in the filehandle change
over time.  The old filehandles are valid indefinitely, until the file
is deleted and hence can be considered permanent and not volatile.
What you say above, however, contradicts the NFS protocol as I understand
it. Here's some relevant text from the latest NFSv4.1 draft (the text in
4.2.1 below exists in in a similar form also in NFSv3, rfc1813)

| 4.2.1.  General Properties of a Filehandle
| ...
| If two filehandles from the same server are equal, they MUST refer to
| the same file. Servers SHOULD try to maintain a one-to-one correspondence
| between filehandles and files but this is not required. Clients MUST use
| filehandle comparisons only to improve performance, not for correct behavior
| All clients need to be prepared for situations in which it cannot be
| determined whether two filehandles denote the same object and in such cases,
| avoid making invalid assumptions which might cause incorrect behavior.
| Further discussion of filehandle and attribute comparison in the context of
| data caching is presented in the section "Data Caching and File Identity".
...
| 9.3.4.  Data Caching and File Identity
| ...
|  When clients cache data, the file data needs to be organized according to
| the file system object to which the data belongs. For NFS version 3 clients,
| the typical practice has been to assume for the purpose of caching that
| distinct filehandles represent distinct file system objects. The client then
| has the choice to organize and maintain the data cache on this basis.
|
| In the NFS version 4 protocol, there is now the possibility to have
| significant deviations from a "one filehandle per object" model because a
| filehandle may be constructed on the basis of the object's pathname.
| Therefore, clients need a reliable method to determine if two filehandles
| designate the same file system object. If clients were simply to assume that
| all distinct filehandles denote distinct objects and proceed to do data
| caching on this basis, caching inconsistencies would arise between the
| distinct client side objects which mapped to the same server side object.
|
| By providing a method to differentiate filehandles, the NFS version 4
| protocol alleviates a potential functional regression in comparison with the
| NFS version 3 protocol. Without this method, caching inconsistencies within
| the same client could occur and this has not been present in previous
| versions of the NFS protocol. Note that it is possible to have such
| inconsistencies with applications executing on multiple clients but that is
| not the issue being addressed here.
|
| For the purposes of data caching, the following steps allow an NFS version 4
| client to determine whether two distinct filehandles denote the same server
| side object:
|
| * If GETATTR directed to two filehandles returns different values of the
|   fsid attribute, then the filehandles represent distinct objects.
| * If GETATTR for any file with an fsid that matches the fsid of the two
|   filehandles in question returns a unique_handles attribute with a value
|   of TRUE, then the two objects are distinct.
| * If GETATTR directed to the two filehandles does not return the fileid
|   attribute for both of the handles, then it cannot be determined whether
|   the two objects are the same. Therefore, operations which depend on that
|   knowledge (e.g. client side data caching) cannot be done reliably.
| * If GETATTR directed to the two filehandles returns different values for
|   the fileid attribute, then they are distinct objects.
| * Otherwise they are the same object.

Even for NFSv3 (that doesn't have the unique_handles attribute I think
that the linux nfs client can do a better job.  If you'd have a filehandle
cache that points at inodes you could maintain a many to one relationship
from multiple filehandles into one inode.  When you discover a new filehandle
you can look up the inode cache for the same fileid and if one is found you
can do a getattr on the old filehandle (without loss of generality you should 
always use the latest filehandle that was returned for that filesystem object,
although any filehandle that refers to it can be used).
If the getattr succeeded then 

RE: Finding hardlinks

2006-12-31 Thread Halevy, Benny
Trond Myklebust wrote:
  
 On Thu, 2006-12-28 at 17:12 +0200, Benny Halevy wrote:
 
  As an example, some file systems encode hint information into the filehandle
  and the hints may change over time, another example is encoding parent
  information into the filehandle and then handles representing hard links
  to the same file from different directories will differ.
 
 Both these examples are bogus. Filehandle information should not change
 over time (except in the special case of NFSv4 volatile filehandles)
 and they should definitely not encode parent directory information that
 can change over time (think rename()!).
 
 Cheers
   Trond
 

The first one is a real life example.  Hints in the filehandle change
over time.  The old filehandles are valid indefinitely, until the file
is deleted and hence can be considered permanent and not volatile.
What you say above, however, contradicts the NFS protocol as I understand
it. Here's some relevant text from the latest NFSv4.1 draft (the text in
4.2.1 below exists in in a similar form also in NFSv3, rfc1813)

| 4.2.1.  General Properties of a Filehandle
| ...
| If two filehandles from the same server are equal, they MUST refer to
| the same file. Servers SHOULD try to maintain a one-to-one correspondence
| between filehandles and files but this is not required. Clients MUST use
| filehandle comparisons only to improve performance, not for correct behavior
| All clients need to be prepared for situations in which it cannot be
| determined whether two filehandles denote the same object and in such cases,
| avoid making invalid assumptions which might cause incorrect behavior.
| Further discussion of filehandle and attribute comparison in the context of
| data caching is presented in the section Data Caching and File Identity.
...
| 9.3.4.  Data Caching and File Identity
| ...
|  When clients cache data, the file data needs to be organized according to
| the file system object to which the data belongs. For NFS version 3 clients,
| the typical practice has been to assume for the purpose of caching that
| distinct filehandles represent distinct file system objects. The client then
| has the choice to organize and maintain the data cache on this basis.
|
| In the NFS version 4 protocol, there is now the possibility to have
| significant deviations from a one filehandle per object model because a
| filehandle may be constructed on the basis of the object's pathname.
| Therefore, clients need a reliable method to determine if two filehandles
| designate the same file system object. If clients were simply to assume that
| all distinct filehandles denote distinct objects and proceed to do data
| caching on this basis, caching inconsistencies would arise between the
| distinct client side objects which mapped to the same server side object.
|
| By providing a method to differentiate filehandles, the NFS version 4
| protocol alleviates a potential functional regression in comparison with the
| NFS version 3 protocol. Without this method, caching inconsistencies within
| the same client could occur and this has not been present in previous
| versions of the NFS protocol. Note that it is possible to have such
| inconsistencies with applications executing on multiple clients but that is
| not the issue being addressed here.
|
| For the purposes of data caching, the following steps allow an NFS version 4
| client to determine whether two distinct filehandles denote the same server
| side object:
|
| * If GETATTR directed to two filehandles returns different values of the
|   fsid attribute, then the filehandles represent distinct objects.
| * If GETATTR for any file with an fsid that matches the fsid of the two
|   filehandles in question returns a unique_handles attribute with a value
|   of TRUE, then the two objects are distinct.
| * If GETATTR directed to the two filehandles does not return the fileid
|   attribute for both of the handles, then it cannot be determined whether
|   the two objects are the same. Therefore, operations which depend on that
|   knowledge (e.g. client side data caching) cannot be done reliably.
| * If GETATTR directed to the two filehandles returns different values for
|   the fileid attribute, then they are distinct objects.
| * Otherwise they are the same object.

Even for NFSv3 (that doesn't have the unique_handles attribute I think
that the linux nfs client can do a better job.  If you'd have a filehandle
cache that points at inodes you could maintain a many to one relationship
from multiple filehandles into one inode.  When you discover a new filehandle
you can look up the inode cache for the same fileid and if one is found you
can do a getattr on the old filehandle (without loss of generality you should 
always use the latest filehandle that was returned for that filesystem object,
although any filehandle that refers to it can be used).
If the getattr succeeded then the filehandles refer to 

RE: [nfsv4] RE: Finding hardlinks

2006-12-31 Thread Halevy, Benny
Trond Myklebust wrote:
  
 On Thu, 2006-12-28 at 15:07 -0500, Halevy, Benny wrote:
  Mikulas Patocka wrote:
 
  BTW. how does (or how should?) NFS client deal with cache coherency if 
  filehandles for the same file differ?
  
  
  Trond can probably answer this better than me...
  As I read it, currently the nfs client matches both the fileid and the
  filehandle (in nfs_find_actor). This means that different filehandles
  for the same file would result in different inodes :(.
  Strictly following the nfs protocol, comparing only the fileid should
  be enough IF fileids are indeed unique within the filesystem.
  Comparing the filehandle works as a workaround when the exported filesystem
  (or the nfs server) violates that.  From a user stand point I think that
  this should be configurable, probably per mount point.
 
 Matching files by fileid instead of filehandle is a lot more trouble
 since fileids may be reused after a file has been deleted. Every time
 you look up a file, and get a new filehandle for the same fileid, you
 would at the very least have to do another GETATTR using one of the
 'old' filehandles in order to ensure that the file is the same object as
 the one you have cached. Then there is the issue of what to do when you
 open(), read() or write() to the file: which filehandle do you use, are
 the access permissions the same for all filehandles, ...
 
 All in all, much pain for little or no gain.

See my answer to your previous reply.  It seems like the current
implementation is in violation of the nfs protocol and the extra pain
is required.

 
 Most servers therefore take great pains to ensure that clients can use
 filehandles to identify inodes. The exceptions tend to be broken in
 other ways

This is true maybe in linux, but not necessarily in non-linux based nfs
servers.

 (Note: knfsd without the no_subtree_check option is one of
 these exceptions - it can break in the case of cross-directory renames).
 
 Cheers,
   Trond


-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: Finding hardlinks

2006-12-28 Thread Halevy, Benny
Mikulas Patocka wrote:
> 
>>> This sounds like a bug to me. It seems like we should have a one to one
>>> correspondence of filehandle -> inode. In what situations would this not be 
>>> the
>>> case?
>>
>> Well, the NFS protocol allows that [see rfc1813, p. 21: "If two file handles 
>> from
>> the same server are equal, they must refer to the same file, but if they are 
>> not
>> equal, no conclusions can be drawn."]
>>
>> As an example, some file systems encode hint information into the filehandle
>> and the hints may change over time, another example is encoding parent
>> information into the filehandle and then handles representing hard links
>> to the same file from different directories will differ.
>
>BTW. how does (or how should?) NFS client deal with cache coherency if 
>filehandles for the same file differ?
>

Trond can probably answer this better than me...
As I read it, currently the nfs client matches both the fileid and the
filehandle (in nfs_find_actor). This means that different filehandles
for the same file would result in different inodes :(.
Strictly following the nfs protocol, comparing only the fileid should
be enough IF fileids are indeed unique within the filesystem.
Comparing the filehandle works as a workaround when the exported filesystem
(or the nfs server) violates that.  From a user stand point I think that
this should be configurable, probably per mount point.

>Mikulas
>
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


RE: Finding hardlinks

2006-12-28 Thread Halevy, Benny
Mikulas Patocka wrote:
 
 This sounds like a bug to me. It seems like we should have a one to one
 correspondence of filehandle - inode. In what situations would this not be 
 the
 case?

 Well, the NFS protocol allows that [see rfc1813, p. 21: If two file handles 
 from
 the same server are equal, they must refer to the same file, but if they are 
 not
 equal, no conclusions can be drawn.]

 As an example, some file systems encode hint information into the filehandle
 and the hints may change over time, another example is encoding parent
 information into the filehandle and then handles representing hard links
 to the same file from different directories will differ.

BTW. how does (or how should?) NFS client deal with cache coherency if 
filehandles for the same file differ?


Trond can probably answer this better than me...
As I read it, currently the nfs client matches both the fileid and the
filehandle (in nfs_find_actor). This means that different filehandles
for the same file would result in different inodes :(.
Strictly following the nfs protocol, comparing only the fileid should
be enough IF fileids are indeed unique within the filesystem.
Comparing the filehandle works as a workaround when the exported filesystem
(or the nfs server) violates that.  From a user stand point I think that
this should be configurable, probably per mount point.

Mikulas

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/