Hello
sync_sb_inodes is called twice and both times like:
spin_lock(&inode_lock);
sync_sb_inodes(sb, wbc);
spin_unlock(&inode_lock);
This patch makes generic_sync_sb_inodes to spin lock itself.
Please, apply this patch. It helps reiser4 to get rid of some oddities.
On Tue, 2005-05-10 at 19:00 +0400, Vladimir Saveliev wrote:
> This patch makes generic_sync_sb_inodes to spin lock itself.
> Please, apply this patch. It helps reiser4 to get rid of some oddities.
>
> [snip]
>
> {
> const unsigned long start = jiffies; /* livelock avoidance */
>
> + spin_lock(&inode_lock);
Looking at what jiffies is used for, it is probably is not a big deal,
but you should move the assignment of start to after acquiring the lock,
as that could take quite some time.
Robert Love
Hello
On Tue, 2005-05-10 at 19:35, Robert Love wrote:
> On Tue, 2005-05-10 at 19:00 +0400, Vladimir Saveliev wrote:
>
> > This patch makes generic_sync_sb_inodes to spin lock itself.
> > Please, apply this patch. It helps reiser4 to get rid of some oddities.
> >
> > [snip]
> >
> > {
> > const unsigned long start = jiffies; /* livelock avoidance */
> >
> > + spin_lock(&inode_lock);
>
> Looking at what jiffies is used for, it is probably is not a big deal,
> but you should move the assignment of start to after acquiring the lock,
> as that could take quite some time.
>
I did not want to un-const start. It would be required for the
assignment move, wouldn't it?
> Robert Love
>
>
>
On Wed, 2005-05-11 at 11:37 +0400, Vladimir Saveliev wrote:
> I did not want to un-const start. It would be required for the
> assignment move, wouldn't it?
Well, the const is just a programming convention. It is useful here,
but just a convention; removing it changes nothing behavior-wise. Your
patch, though, changes behavior.
How bad do you need to push the spin locks into the function?
Robert Love
Hello
On Wed, 2005-05-11 at 21:49, Robert Love wrote:
> On Wed, 2005-05-11 at 11:37 +0400, Vladimir Saveliev wrote:
>
> > I did not want to un-const start. It would be required for the
> > assignment move, wouldn't it?
>
> Well, the const is just a programming convention. It is useful here,
> but just a convention; removing it changes nothing behavior-wise. Your
> patch, though, changes behavior.
>
ok, I will move assignment.
> How bad do you need to push the spin locks into the function?
>
The reason is that reiser4 implements its own sync_inodes method of
struct super_operations. reiser4_sync_inodes first calls
generic_sync_sb_inodes and then calls reiser4' function to flush atoms
to disk. If generic_sync_sb_inodes would exit with inode_lock locked,
reiser4_sync_inodes would have to unlock inode_lock after
generic_sync_sb_inodes and lock it before exit. inode_lock is static for
fs/inode.c, so, we asked whether it would be possible to have
spinlocking in generic_sync_sb_inodes.