Re: [PATCH 04/16 v4] pramfs: file operations
2010/11/24 Paul Mundt : > On Wed, Nov 24, 2010 at 09:11:13AM +0100, Marco Stornelli wrote: >> 2010/11/24 Paul Mundt : >> > most of this from ext2, I'm curious why you opted to hardcode this >> > instead of maintaining the flexibility that ext2 XIP has over this. >> >> First of all because it was simpler :) In addition there was some >> design problem to use it in combination with the memory protection. > > Do you have more details on this? You can easily check for unsupportable > configurations with mount options and bail out accordingly. > >> The difference with ext2 is that we aren't talking about a general >> purpose fs used (mainly) on normal desktop/server systems, but a >> specific fs for embedded world, so I think some little constraints are >> ok. >> > I'm not sure what your point is. It's not a general purpose file system, > but that's not an excuse for taking shortcuts. Out of the boards on my > desk, I have at least 3 that could make use of this file system where I > could use both XIP and non-XIP for different stores out of the box. I > wouldn't exactly call it a corner case. Also, as Tony's patch set > demonstrates, these sorts of non-volatile data stores are common enough > in the server space to make pramfs an option there, too. Please lose this > mentality that because something was originally tasked for embedded it's > perfectly acceptable to ship a crippled interface. I'm not responsible if someone uses something outside its scope, you can use FAT on a flash but you can't claim reliability. However since I'm a collaborative person I'll try to fix it, implementing where possible a mount option. > > As it is, this is something that will have to be rewritten one way or the > other, but whether that happens in or out of staging/ is not such a big > concern. > Good. Marco -- To unsubscribe from this list: send the line "unsubscribe linux-embedded" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 04/16 v4] pramfs: file operations
On Wed, Nov 24, 2010 at 09:11:13AM +0100, Marco Stornelli wrote: > 2010/11/24 Paul Mundt : > > most of this from ext2, I'm curious why you opted to hardcode this > > instead of maintaining the flexibility that ext2 XIP has over this. > > First of all because it was simpler :) In addition there was some > design problem to use it in combination with the memory protection. Do you have more details on this? You can easily check for unsupportable configurations with mount options and bail out accordingly. > The difference with ext2 is that we aren't talking about a general > purpose fs used (mainly) on normal desktop/server systems, but a > specific fs for embedded world, so I think some little constraints are > ok. > I'm not sure what your point is. It's not a general purpose file system, but that's not an excuse for taking shortcuts. Out of the boards on my desk, I have at least 3 that could make use of this file system where I could use both XIP and non-XIP for different stores out of the box. I wouldn't exactly call it a corner case. Also, as Tony's patch set demonstrates, these sorts of non-volatile data stores are common enough in the server space to make pramfs an option there, too. Please lose this mentality that because something was originally tasked for embedded it's perfectly acceptable to ship a crippled interface. As it is, this is something that will have to be rewritten one way or the other, but whether that happens in or out of staging/ is not such a big concern. -- To unsubscribe from this list: send the line "unsubscribe linux-embedded" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 04/16 v4] pramfs: file operations
2010/11/24 Paul Mundt :
> On Sat, Nov 20, 2010 at 10:58:40AM +0100, Marco Stornelli wrote:
>> diff -Nurp linux-2.6.36-orig/fs/pramfs/file.c linux-2.6.36/fs/pramfs/file.c
>> --- linux-2.6.36-orig/fs/pramfs/file.c 1970-01-01 01:00:00.0
>> +0100
>> +++ linux-2.6.36/fs/pramfs/file.c 2010-09-24 18:34:03.0 +0200
>> @@ -0,0 +1,166 @@
>> +/*
>> + * FILE NAME fs/pramfs/file.c
>> + *
>> + * BRIEF DESCRIPTION
>> + *
>> + * File operations for files.
>> + *
> This FILE NAME / BRIEF DESCRIPTION thing should probably die, it's also
> not used consistently throughout the series.
Maybe yes, it's more a problem then a advantage :)
>
>> +static int pram_open_file(struct inode *inode, struct file *filp)
>> +{
>> +#ifndef CONFIG_PRAMFS_XIP
>> + /* Without XIP we force to use Direct IO */
>> + filp->f_flags |= O_DIRECT;
>> +#endif
>> + return generic_file_open(inode, filp);
>> +}
>> +
> Relying on a config option for this is in violently poor taste. Rely on
> the config option to enable/disable XIP support as you like, but whether
> it's actually mounted XIP or not should really depend be determined by a
> mount option. Presently you have no way of dealing with the file system
> being mounted multiple times with mixed XIP and non-XIP, which doesn't
> seem like a particularly exotic configuration. As you seem to have copied
Maybe on embedded, it could be.
> most of this from ext2, I'm curious why you opted to hardcode this
> instead of maintaining the flexibility that ext2 XIP has over this.
>
First of all because it was simpler :) In addition there was some
design problem to use it in combination with the memory protection.
The difference with ext2 is that we aren't talking about a general
purpose fs used (mainly) on normal desktop/server systems, but a
specific fs for embedded world, so I think some little constraints are
ok.
Marco
--
To unsubscribe from this list: send the line "unsubscribe linux-embedded" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 04/16 v4] pramfs: file operations
On Sat, Nov 20, 2010 at 10:58:40AM +0100, Marco Stornelli wrote:
> diff -Nurp linux-2.6.36-orig/fs/pramfs/file.c linux-2.6.36/fs/pramfs/file.c
> --- linux-2.6.36-orig/fs/pramfs/file.c1970-01-01 01:00:00.0
> +0100
> +++ linux-2.6.36/fs/pramfs/file.c 2010-09-24 18:34:03.0 +0200
> @@ -0,0 +1,166 @@
> +/*
> + * FILE NAME fs/pramfs/file.c
> + *
> + * BRIEF DESCRIPTION
> + *
> + * File operations for files.
> + *
This FILE NAME / BRIEF DESCRIPTION thing should probably die, it's also
not used consistently throughout the series.
> +static int pram_open_file(struct inode *inode, struct file *filp)
> +{
> +#ifndef CONFIG_PRAMFS_XIP
> + /* Without XIP we force to use Direct IO */
> + filp->f_flags |= O_DIRECT;
> +#endif
> + return generic_file_open(inode, filp);
> +}
> +
Relying on a config option for this is in violently poor taste. Rely on
the config option to enable/disable XIP support as you like, but whether
it's actually mounted XIP or not should really depend be determined by a
mount option. Presently you have no way of dealing with the file system
being mounted multiple times with mixed XIP and non-XIP, which doesn't
seem like a particularly exotic configuration. As you seem to have copied
most of this from ext2, I'm curious why you opted to hardcode this
instead of maintaining the flexibility that ext2 XIP has over this.
--
To unsubscribe from this list: send the line "unsubscribe linux-embedded" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 04/16 v4] pramfs: file operations
From: Marco Stornelli
File operations.
Signed-off-by: Marco Stornelli
---
diff -Nurp linux-2.6.36-orig/fs/pramfs/file.c linux-2.6.36/fs/pramfs/file.c
--- linux-2.6.36-orig/fs/pramfs/file.c 1970-01-01 01:00:00.0 +0100
+++ linux-2.6.36/fs/pramfs/file.c 2010-09-24 18:34:03.0 +0200
@@ -0,0 +1,166 @@
+/*
+ * FILE NAME fs/pramfs/file.c
+ *
+ * BRIEF DESCRIPTION
+ *
+ * File operations for files.
+ *
+ * Copyright 2009-2010 Marco Stornelli
+ * Copyright 2003 Sony Corporation
+ * Copyright 2003 Matsushita Electric Industrial Co., Ltd.
+ * 2003-2004 (c) MontaVista Software, Inc. , Steve Longerbeam
+ * This file is licensed under the terms of the GNU General Public
+ * License version 2. This program is licensed "as is" without any
+ * warranty of any kind, whether express or implied.
+ */
+#include
+#include
+#include
+#include
+#include
+#include
+#include "pram.h"
+#include "acl.h"
+#include "xip.h"
+#include "xattr.h"
+
+static int pram_open_file(struct inode *inode, struct file *filp)
+{
+#ifndef CONFIG_PRAMFS_XIP
+ /* Without XIP we force to use Direct IO */
+ filp->f_flags |= O_DIRECT;
+#endif
+ return generic_file_open(inode, filp);
+}
+
+ssize_t __pram_direct_IO(int rw, struct kiocb *iocb,
+ const struct iovec *iov,
+ loff_t offset, unsigned long nr_segs)
+{
+ struct file *file = iocb->ki_filp;
+ struct inode *inode = file->f_mapping->host;
+ struct super_block *sb = inode->i_sb;
+ int progress = 0, hole = 0;
+ ssize_t retval = 0;
+ void *tmp = NULL;
+ unsigned long blocknr, blockoff;
+ int num_blocks, blocksize_mask, blocksize, blocksize_bits;
+ char __user *buf = iov->iov_base;
+ size_t length = iov_length(iov, nr_segs);
+
+ if (length < 0)
+ return -EINVAL;
+ if ((rw == READ) && (offset + length > inode->i_size))
+ length = inode->i_size - offset;
+ if (!length)
+ goto out;
+
+ blocksize_bits = inode->i_sb->s_blocksize_bits;
+ blocksize = 1 << blocksize_bits;
+ blocksize_mask = blocksize - 1;
+
+ /* find starting block number to access */
+ blocknr = offset >> blocksize_bits;
+ /* find starting offset within starting block */
+ blockoff = offset & blocksize_mask;
+ /* find number of blocks to access */
+ num_blocks = (blockoff + length + blocksize_mask) >> blocksize_bits;
+
+ if (rw == WRITE) {
+ /* prepare a temporary buffer to hold a user data block
+ for writing. */
+ tmp = kmalloc(blocksize, GFP_KERNEL);
+ if (!tmp)
+ return -ENOMEM;
+ /* now allocate the data blocks we'll need */
+ retval = pram_alloc_blocks(inode, blocknr, num_blocks);
+ if (retval)
+ goto fail1;
+ }
+
+ while (length) {
+ int count;
+ u8 *bp = NULL;
+ u64 block = pram_find_data_block(inode, blocknr++);
+ if (unlikely(!block && rw == READ)) {
+ /* We are falling in a hole */
+ hole = 1;
+ } else {
+ bp = (u8 *)pram_get_block(sb, block);
+ if (!bp)
+ goto fail2;
+ }
+
+ count = blockoff + length > blocksize ?
+ blocksize - blockoff : length;
+
+ if (rw == READ) {
+ if (unlikely(hole)) {
+ retval = clear_user(buf, count);
+ if (retval) {
+ retval = -EFAULT;
+ goto fail1;
+ }
+ } else {
+ retval = copy_to_user(buf, &bp[blockoff],
count);
+ if (retval) {
+ retval = -EFAULT;
+ goto fail1;
+ }
+ }
+ } else {
+ retval = copy_from_user(tmp, buf, count);
+ if (retval) {
+ retval = -EFAULT;
+ goto fail1;
+ }
+
+ pram_memunlock_block(inode->i_sb, bp);
+ memcpy(&bp[blockoff], tmp, count);
+ pram_memlock_block(inode->i_sb, bp);
+ }
+
+ progress += count;
+ buf += count;
+ length -= count;
+ blockoff = 0;
+ hole = 0;
+ }
+
+fail2:
+ retval = progress;
+fail1:
+ kfree(tmp);
+out:
+ return retval;
+}
+
+int __pram_mmap(struct file *file, struct vm_area_struct *vma)
+{
+ /* Only pr
