Re: question on fs/read_write.c modification from 2.6.7 to 2.6.8.1

Re: question on fs/read_write.c modification from 2.6.7 to 2.6.8.1

[Alan Cox]

On Gwe, 2004-09-10 at 11:45, manjunathg.kondaiah@wipro.com wrote:
> and In the implementation, the driver would be interested in checking
> the sanity of pointers by doing a check like if (ppos !=  &(file->fpos)
> { printk( KERN_ERR "Pointers not matching\n");return -EPERM;}

It now has nonseekable_open() to call.

> 1.  "so that the VFS layer is responsible for updating that offset
> rather than individual drivers." By not passing ppos as file->fops, the
> drivers should not try and do a (*ppos)++ anymore. Well the determined
> rogue driver can still do a file->fpos++ coz the file structure is still
> being exposed by the kernel to the driver (verified with printks). So
> this option does not sound logical!

This is the reason - plus better enforcement of pread/pwrite return
values. Any code touching file->f_pos in a driver is generally broken
anyway. It doesn't prevent such use but it makes the default behaviour
for a driver correct so makes it easier to write drivers correctly

question on fs/read_write.c modification from 2.6.7 to 2.6.8.1

[manjunathg.kondaiah]

Hi All,

As per the patch provided to the Bit
keeper(http://linux.bkbits.net:8080/linux-2.6/hist/fs/read_write.c?nav=i
ndex.html|src/|src/fs), the fs/read_write.c has been modified, to quote
"so that the VFS layer is responsible for updating that offset rather
than individual drivers." I am having trouble to understand the logic
for this change.

Firstly, till 2.6.7 any driver implementation of a read or write would
have been :

int driver_read(struct file *file, char *buffer, size_t count, loff_t *
ppos){...}

and In the implementation, the driver would be interested in checking
the sanity of pointers by doing a check like if (ppos !=  &(file->fpos)
{ printk( KERN_ERR "Pointers not matching\n");return -EPERM;}

This is no longer possible. But I am unable to understand the rationale
behind this decision. Some of the possible reasons I discarded are:

1.  "so that the VFS layer is responsible for updating that offset
rather than individual drivers." By not passing ppos as file->fops, the
drivers should not try and do a (*ppos)++ anymore. Well the determined
rogue driver can still do a file->fpos++ coz the file structure is still
being exposed by the kernel to the driver (verified with printks). So
this option does not sound logical!

2.  Is some sort of optimization taking place by doing this? From the
patch:

-               ret = vfs_read(file, buf, count, &file->f_pos);
+               loff_t pos = file_pos_read(file);
+               ret = vfs_read(file, buf, count, &pos);
+               file_pos_write(file, pos);

For this specific example, the machine instructions would not get
reduced in any way. Arm code wise (objdump), well the function sys_read
increased in size from 54 bytes(2.6.7) to 70 bytes(2.6.8.1)! So I guess
the logic of optimizing the code does not hold good here.

I am unable to think of any other reasons. Could some one guide me in
this regards?

Regards,
Manjunath

RE: question on fs/read_write.c modification from 2.6.7 to 2.6.8.1

[manjunathg.kondaiah]

Hi Alan,
Thanks for the clarifications. Just one more set of clarifications:
>It doesn't prevent such use but it makes the default behaviour
>for a driver correct so makes it easier to write drivers correctly
1. Is the performance penalty we pay for every read and write worth
this? I assume it does.
2. Would it provide better enforcement if the file structure is also a
copied when passed to drivers? A kind of encapsulation...
Regards,
Manjunath