lkml.org 
[lkml]   [2008]   [Jun]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    SubjectRe: [RFC][PATCH 4/4] configfs: Make multiple default_group destructions lockdep friendly
    On Mon, Jun 02, 2008 at 04:07:21PM -0700, Joel Becker wrote:
    > A couple comments.
    > First, put a BUG_ON() where you have BAD BAD BAD - we shouldn't
    > be creating a depth we can't delete.

    I think that the best way to avoid this is to use the same numbering scheme
    while attaching default groups.

    This would change the body of populate_groups() like this:

    - if (group->default_groups) {
    + /* lock_level starts at zero for the non default group.
    + * Set it even if we do not take the lock, so that we can use the same
    + * numbering scheme as for destruction time, and can prevent overload at
    + * destruction time. */
    + lock_level = set_dirent_lock_level(parent_sd, sd);
    + if (lock_level < 0) {
    + /* Too many default groups */
    + ret = lock_level;
    + } else if (group->default_groups) {
    /*
    * FYI, we're faking mkdir here
    * I'm not sure we need this semaphore, as we're called
    * from our parent's mkdir. That holds our parent's
    * i_mutex, so afaik lookup cannot continue through our
    * parent to find us, let alone mess with our tree.
    * That said, taking our i_mutex is closer to mkdir
    * emulation, and shouldn't hurt.
    */
    - /* lock_level starts at zero for the non default group */
    - lock_level = set_dirent_lock_level(parent_sd, sd);
    - if (lock_level < 0) {
    - /* Too deeply nested default groups */
    - ret = lock_level;
    - } else {
    mutex_lock_nested(&dentry->d_inode->i_mutex,
    I_MUTEX_CHILD + lock_level);

    for (i = 0; group->default_groups[i]; i++) {
    new_group = group->default_groups[i];

    ret = create_default_group(group, new_group);
    if (ret)
    break;
    }

    mutex_unlock(&dentry->d_inode->i_mutex);
    - /* Reset for future sub-group creations */
    - reset_dirent_lock_level(sd);
    - }
    }
    + if (lock_level > 0)
    + /* Update parent lock_level to keep it increasing, but not
    + * if group is the one actually created/registered by the
    + * user/subsystem */
    + copy_dirent_lock_level(sd, parent_sd);
    + /* Reset for future sub-group creations */
    + reset_dirent_lock_level(sd);

    >
    > > @@ -392,6 +437,10 @@ static int configfs_detach_prep(struct d
    > > * deep nesting of default_groups
    > > */
    > > ret = configfs_detach_prep(sd->s_dentry);
    > > + /* Update parent's lock_level so that remaining
    > > + * sibling children keep on globally increasing
    > > + * lock_level */
    > > + copy_dirent_lock_level(sd, parent_sd);
    > > if (!ret)
    > > continue;
    > > } else
    >
    > I'm not sure I get this hunk. If our parent was 1 and we are 2,
    > we are copying 2 to our parent so the parent can only have other
    > children at 3?

    Exactly.

    Louis

    --
    Dr Louis Rilling Kerlabs
    Skype: louis.rilling Batiment Germanium
    Phone: (+33|0) 6 80 89 08 23 80 avenue des Buttes de Coesmes
    http://www.kerlabs.com/ 35700 Rennes

    \
     
     \ /
      Last update: 2008-06-03 18:03    [W:0.026 / U:60.124 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site