[ECOS] A question about fileio, passing an empty string argument to chdir's problem

[ECOS] A question about fileio, passing an empty string argument to chdir's problem

[eCos]

Dear all:

We think there is a problem with eCos' chdir. When this fileio's function 
is passed an empty 

string as its argument, eCos' chdir does nothing and will not fail. 
Calling chdir("") seems to 

have the same effect as chdir(".").

However, as far as POSIX standard is concerned, calling chdir("") should 
fail and return ENOENT, 

 just as described below(from POSIX document), please NOTE the contents in 
the stars:

------------------------------------------------------------------------------------------------
The chdir() function shall fail if: 
[EACCES] 
Search permission is denied for any component of the pathname. 
[ELOOP] 
A loop exists in symbolic links encountered during resolution of the path 
argument. 
[ENAMETOOLONG] 
The length of the path argument exceeds {PATH_MAX} or a pathname component 
is longer than 

{NAME_MAX}. 
***********************************************************************************************
[ENOENT] 
A component of path does not name an existing directory or path is an 
empty string. 
***********************************************************************************************
[ENOTDIR] 
A component of the pathname is not a directory. 
------------------------------------------------------------------------------------------------

We assume that eCos is in accord with POSIX as a matter of course, and we 
think maybe this 

problem can be resolved by adding a check condition to cyg_mtab_lookup 
function,  cause chdir

(open, mkdir, etc. are the same) will call cyg_mtab_lookup()

------------------------------------------------------------------------------------------------
1  __externC int cyg_mtab_lookup( cyg_dir *dir, const char **name, 
cyg_mtab_entry **mte) 
2  { 
3    cyg_mtab_entry *m, *best = NULL; 
4    int best_len = 0; 
5 
6    // Unrooted file names start from current dir 
7    if( **name != '/' ) { 
8        int cwd_len; 
9        if (*mte == (cyg_mtab_entry *)NULL) { 
10           // No known current directory 
11           return -1; 
12       } 
13   ...
14  } 
------------------------------------------------------------------------------------------------

we add to line 5

------------------------------------------------------------------------------------------------
3    cyg_mtab_entry *m, *best = NULL; 
4    int best_len = 0; 
5 
     if (**name == '\0')        // if an empty string, then **name == '\0'
         return -1;
6    // Unrooted file names start from current dir 
7    if( **name != '/' ) { 
8       int cwd_len; 
9       if (*mte == (cyg_mtab_entry *)NULL) { 
10           // No known current directory 
11           return -1; 
12       } 
13    ...
14    } 
------------------------------------------------------------------------------------------------

But we are not sure whether we are correct, or maybe eCos has some deep 
consideration, so could 

you please examine this, then tell us whether we are right or explain for 
us, thank you so much.


Sincerely Yours


***********************************************
Beijing Sunnorth eCos Maintainer Group

Maintainers:
liqin@sunnorth.com.cn
wanghui@sunnorth.com.cn
taiyun@sunnorth.com.cn
yxinghua@sunnorth.com.cn

Bejing Sunnorth Electronic Technology Co.,LTD
***********************************************


***********************************************
Beijing Sunnorth eCos Maintainer Group

Maintainers:
liqin@sunnorth.com.cn
wanghui@sunnorth.com.cn
taiyun@sunnorth.com.cn
yxinghua@sunnorth.com.cn

Bejing Sunnorth Electronic Technology Co.,LTD
***********************************************

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] A question about fileio, passing an empty string argument to chdir's problem

[Andrew Lunn]

On Wed, Feb 27, 2008 at 06:06:43PM +0800, eCos@sunnorth.com.cn wrote:
> Dear all:
> 
> We think there is a problem with eCos' chdir. When this fileio's function 
> is passed an empty 

Hi Beijing Sunnorth eCos Maintainer Group

I received this email 5 times! Please think about which addresses you
send to, there is no need to spam everybody. A single email to
ecos-discuss would of been sufficient.

       Andrew

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] A question about fileio, passing an empty string argument to chdir's problem

[Andrew Lunn]

On Wed, Feb 27, 2008 at 06:06:43PM +0800, eCos@sunnorth.com.cn wrote:
> Dear all:
> 
> We think there is a problem with eCos' chdir. When this fileio's function 
> is passed an empty 
> 
> string as its argument, eCos' chdir does nothing and will not fail. 
> Calling chdir("") seems to 
> 
> have the same effect as chdir(".").
> 
> However, as far as POSIX standard is concerned, calling chdir("") should 
> fail and return ENOENT, 
> 
>  just as described below(from POSIX document), please NOTE the contents in 
> the stars:
> 
> ------------------------------------------------------------------------------------------------
> The chdir() function shall fail if: 
> [EACCES] 
> Search permission is denied for any component of the pathname. 
> [ELOOP] 
> A loop exists in symbolic links encountered during resolution of the path 
> argument. 
> [ENAMETOOLONG] 
> The length of the path argument exceeds {PATH_MAX} or a pathname component 
> is longer than 
> 
> {NAME_MAX}. 
> ***********************************************************************************************
> [ENOENT] 
> A component of path does not name an existing directory or path is an 
> empty string. 
> ***********************************************************************************************
> [ENOTDIR] 
> A component of the pathname is not a directory. 
> ------------------------------------------------------------------------------------------------
> 
> We assume that eCos is in accord with POSIX as a matter of course, and we 
> think maybe this 
> 
> problem can be resolved by adding a check condition to cyg_mtab_lookup 
> function,  cause chdir
> 
> (open, mkdir, etc. are the same) will call cyg_mtab_lookup()

I just checked the man pages for open and mkdir. Only chdir returns
ENOENT for a "" path. Also cyg_mtab_lookup() just finds the mount
point of a filesystem which contains the given path. To me i don't see
why it should be making checks which only apply to chdir().

I would put the check in chdir() itself.

Please send me a patch for this and also extend some of the test cases
which use chdir to test for chdir("") returns ENOENT.

As a side note. You are probably aware of this already, but remember
eCos is thread based, not process based. If a thread uses chdir() it
affects all threads. If you are porting Unix code which was originally
designed to run in a process, not a thread, you might get into
trouble.

        Andrew

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

[ECOS] blib node allocation problem

[Steve West]

Hi all,
    I am having some problems with blib node allocation. It seems that the 
number of unique file names is controlled by node pool size. If I set 
NODE_POOL_SIZE to 32k and then open and close 32k unique named files I get a 
file open failure around 32k files. Any ideas?

Steve. 


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] blib node allocation problem

[Andrew Lunn]

On Wed, Feb 27, 2008 at 09:06:42AM -0500, Steve West wrote:
> Hi all,
>    I am having some problems with blib node allocation. It seems that the 
> number of unique file names is controlled by node pool size. If I set  
> NODE_POOL_SIZE to 32k and then open and close 32k unique named files I 
> get a file open failure around 32k files. Any ideas?

Could you produce a little test case which demonstrates the problem?
If you follow the usual test case style i will include it as a
standard test so that once we have the problem fixed we can also use
it for regression testing.

      Thank       
           Andrew

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] blib node allocation problem with FAT FS

[Steve West]

> On Wed, Feb 27, 2008 at 09:06:42AM -0500, Steve West wrote:
>> Hi all,
>>    I am having some problems with blib node allocation. It seems that the
>> number of unique file names is controlled by node pool size. If I set
>> NODE_POOL_SIZE to 32k and then open and close 32k unique named files I
>> get a file open failure around 32k files. Any ideas?
>
> Could you produce a little test case which demonstrates the problem?
> If you follow the usual test case style i will include it as a
> standard test so that once we have the problem fixed we can also use
> it for regression testing.
>
I found the problem. The reference count was decrementing below 0. Here is a 
fix for fatfs_node_unref() in fatfs_ncache.c.

  if (node->refcnt)
        node->refcnt--;

I also found that if you change the 512 to 32768 (sector size) in fatfs.c 
call to cyg_blib_io_create() you get a 50% - 60% increase in disk 
throughput, especially with large files. (driver reads writes clusters not 
sectors)

Steve


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] blib node allocation problem with FAT FS

[Andrew Lunn]

> I found the problem. The reference count was decrementing below 0. Here 
> is a fix for fatfs_node_unref() in fatfs_ncache.c.
>
>  if (node->refcnt)
>        node->refcnt--;

It could be that you have fixed the symptom and not the cause. Why is
it being unreferenced more times than it is referenced? Maybe there is
a missing ref() so you could get file corrupt?

Before i commit this i want a clear explanation why your change is
correct.

> I also found that if you change the 512 to 32768 (sector size) in fatfs.c 
> call to cyg_blib_io_create() you get a 50% - 60% increase in disk  
> throughput, especially with large files. (driver reads writes clusters 
> not sectors)

Which driver? I quickly looked at the IDE and the MMC driver and both
appear to read/write blocks.

There is a FIXME where the cyg_blib_io_create() is, saying to get the
block size from the device. Maybe you want to implement this?

      Andrew

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] blib node allocation problem with FAT FS

[Steve West]

>> I found the problem. The reference count was decrementing below 0. Here
>> is a fix for fatfs_node_unref() in fatfs_ncache.c.
>>
>>  if (node->refcnt)
>>        node->refcnt--;
>
> It could be that you have fixed the symptom and not the cause. Why is
> it being unreferenced more times than it is referenced? Maybe there is
> a missing ref() so you could get file corrupt?
>
> Before i commit this i want a clear explanation why your change is
> correct.
>
>> I also found that if you change the 512 to 32768 (sector size) in fatfs.c
>> call to cyg_blib_io_create() you get a 50% - 60% increase in disk
>> throughput, especially with large files. (driver reads writes clusters
>> not sectors)
>
> Which driver? I quickly looked at the IDE and the MMC driver and both
> appear to read/write blocks.
>
> There is a FIXME where the cyg_blib_io_create() is, saying to get the
> block size from the device. Maybe you want to implement this?

I think the block change will work with any IDE driver. The IDE Block size 
is 512 but by changing the size to 32768 you read write multiple blocks at 
one time. This makes the throughput much faster especially with DMA mode.

As for the fix to fatfs_node_unref() in fatfs_ncache.c, I will check into it 
further.

Steve


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] blib node allocation problem with FAT FS

[Andrew Lunn]

On Wed, Feb 27, 2008 at 06:56:07PM +0100, Andrew Lunn wrote:
> > I found the problem. The reference count was decrementing below 0. Here 
> > is a fix for fatfs_node_unref() in fatfs_ncache.c.
> >
> >  if (node->refcnt)
> >        node->refcnt--;
> 
> It could be that you have fixed the symptom and not the cause. Why is
> it being unreferenced more times than it is referenced? Maybe there is
> a missing ref() so you could get file corrupt?

In fact, i very much suspect this fix is wrong. Look at the code:

void
fatfs_node_unref(fatfs_disk_t *disk, fatfs_node_t *node)
{
    CYG_CHECK_DATA_PTRC(disk);
    CYG_CHECK_DATA_PTRC(node);
    CYG_TRACE2(TNC, "node='%s' refcnt=%d", node->dentry.filename, node->refcnt);
    CYG_ASSERT(node->refcnt > 0, "node->refcnt <= 0");

i.e, if you had asserts enabled, it would of thrown an assert failure.

You need to dig further and find the real problem. Turning on asserts
might make this easier to find.

    Andrew

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] blib node allocation problem with FAT FS

[Andrew Lunn]

> I think the block change will work with any IDE driver. The IDE Block 
> size is 512 but by changing the size to 32768 you read write multiple 
> blocks at one time. This makes the throughput much faster especially with 
> DMA mode.

It would make sequential access faster. But you have reduced the
number of cache blocks by a factor of 64. So an application which is
doing scattered random writes will suffer since there will be a lot of
cache flushing going on, and most of the writes will be redundant
since only a small number of bytes in the big block have been changed.

So it is a trade off between sequential and random access.

I would suggest making this a CDL option so it can be configured for
your specific needs.

     Andrew

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] blib node allocation problem with FAT FS

[Steve West]

> I would suggest making this a CDL option so it can be configured for
> your specific needs.

Yes that would be good.

As to the other Blib issue. I see the ASSERT for refcnt = 0. But following 
the code this is what I get

open
node = fatfs_node_alloc(disk, &new_file_dentry);
       node->refcnt = 0;

write
 no reference to refcnt

close
fatfs_node_unref(disk, node);
        node->refcnt--;

Seems that there is something basically wrong here.

Steve 


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] blib node allocation problem with FAT FS

[Andrew Lunn]

On Wed, Feb 27, 2008 at 01:45:23PM -0500, Steve West wrote:
>> I would suggest making this a CDL option so it can be configured for
>> your specific needs.
>
> Yes that would be good.
>
> As to the other Blib issue. I see the ASSERT for refcnt = 0. But 
> following the code this is what I get
>
> open
> node = fatfs_node_alloc(disk, &new_file_dentry);
>       node->refcnt = 0;

fatfs.c: 647 should of incremented the reference count. So it should
be 1.  What was the value before line 647?

> Seems that there is something basically wrong here.

Yep...

        Andrew

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] blib node allocation problem with FAT FS

[Steve West]

> fatfs.c: 647 should of incremented the reference count. So it should
> be 1.  What was the value before line 647?
>
>> Seems that there is something basically wrong here.
>
I set the hash table size to the same as the file count size and the problem 
goes away. I think the hash table was too small.

Steve 


-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: [ECOS] blib node allocation problem with FAT FS

[Andrew Lunn]

On Wed, Feb 27, 2008 at 03:25:49PM -0500, Steve West wrote:
>> fatfs.c: 647 should of incremented the reference count. So it should
>> be 1.  What was the value before line 647?
>>
>>> Seems that there is something basically wrong here.
>>
> I set the hash table size to the same as the file count size and the 
> problem goes away. I think the hash table was too small.

How can the hash table be too small? Hashes are allowed to collide and
when they do you get a linked list.

Again you have fixed the symptom and not the cause. Don't be surprised
if this does come back sometime. We need to properly debug this. So
please write a test case so that i can debug it properly, or you debug
it and find the real cause.

   Andrew

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss

Re: Re: [ECOS] A question about fileio, passing an empty string argument to chdir's problem

[eCos]

Dears;

Really sorry for my redundant emails. We thought we only sent the email 1 
time, however, that is not the case. We had received email sending failure 
not for 2 times, so we sent again, maybe that caused the problem, sorry 
again

But I still have a question, I have tested on linux, CentOS5, gcc version 
4.1.1 20070105 (Red Hat 4.1.1-52), calling open("", ...) will also return 
ENOENT(Please see code below). So I write this mail to affirm it, could 
you please look over this? Thanks so much.



SOURCE CODE
----------------------------------------------------------------
#include <sys/types.h>
#include <sys/stat.h>
#include <fcntl.h>

#include <stdio.h>
#include <string.h>
#include <errno.h>

int
main(void)
{
    int fd;
    fd = open("", O_RDWR | O_CREAT);
    printf("fd:%d\n", fd);
    if(fd < 0)
    printf("open error, errno:%d\n", errno);
    else
    close(fd);
    return 0;
}
----------------------------------------------------------------
OUTPUT
----------------------------------------------------------------
fd:-1
open error, errno:2
----------------------------------------------------------------

***********************************************
Beijing Sunnorth eCos Maintainer Group

Maintainers:
liqin@sunnorth.com.cn
wanghui@sunnorth.com.cn
taiyun@sunnorth.com.cn
yxinghua@sunnorth.com.cn

Bejing Sunnorth Electronic Technology Co.,LTD
***********************************************



Andrew Lunn <andrew@lunn.ch> 
发件人:  ecos-discuss-owner@ecos.sourceware.org
2008-02-27 21:39

收件人
eCos@sunnorth.com.cn
抄送
ecos-discuss@ecos.sourceware.org
主题
Re: [ECOS] A question about fileio, passing an empty string     argument 
to chdir's  problem






On Wed, Feb 27, 2008 at 06:06:43PM +0800, eCos@sunnorth.com.cn wrote:
> Dear all:
> 
> We think there is a problem with eCos' chdir. When this fileio's 
function 
> is passed an empty 
> 
> string as its argument, eCos' chdir does nothing and will not fail. 
> Calling chdir("") seems to 
> 
> have the same effect as chdir(".").
> 
> However, as far as POSIX standard is concerned, calling chdir("") should 

> fail and return ENOENT, 
> 
>  just as described below(from POSIX document), please NOTE the contents 
in 
> the stars:
> 
> 
------------------------------------------------------------------------------------------------
> The chdir() function shall fail if: 
> [EACCES] 
> Search permission is denied for any component of the pathname. 
> [ELOOP] 
> A loop exists in symbolic links encountered during resolution of the 
path 
> argument. 
> [ENAMETOOLONG] 
> The length of the path argument exceeds {PATH_MAX} or a pathname 
component 
> is longer than 
> 
> {NAME_MAX}. 
> 
***********************************************************************************************
> [ENOENT] 
> A component of path does not name an existing directory or path is an 
> empty string. 
> 
***********************************************************************************************
> [ENOTDIR] 
> A component of the pathname is not a directory. 
> 
------------------------------------------------------------------------------------------------
> 
> We assume that eCos is in accord with POSIX as a matter of course, and 
we 
> think maybe this 
> 
> problem can be resolved by adding a check condition to cyg_mtab_lookup 
> function,  cause chdir
> 
> (open, mkdir, etc. are the same) will call cyg_mtab_lookup()

I just checked the man pages for open and mkdir. Only chdir returns
ENOENT for a "" path. Also cyg_mtab_lookup() just finds the mount
point of a filesystem which contains the given path. To me i don't see
why it should be making checks which only apply to chdir().

I would put the check in chdir() itself.

Please send me a patch for this and also extend some of the test cases
which use chdir to test for chdir("") returns ENOENT.

As a side note. You are probably aware of this already, but remember
eCos is thread based, not process based. If a thread uses chdir() it
affects all threads. If you are porting Unix code which was originally
designed to run in a process, not a thread, you might get into
trouble.

        Andrew

-- 
Before posting, please read the FAQ: http://ecos.sourceware.org/fom/ecos
and search the list archive: http://ecos.sourceware.org/ml/ecos-discuss