Re: Finding an available fss device

[Robert Elz <kre%munnari.OZ.AU@localhost>]

I doubt that your new proposed ioctl() is a very good
interface - to do what you really need you would require
the equivalent of a "test & set" otherwise all you are
doing is creating a race condition - even though it is,
because of the low number of users, one that is
unlikely to matter very often.

If the interface were useful, I see no point in taking and
releasing a mutex in order to read one bit - the bit is either
set or not, and is either stable set or not (if it is stable,
the mutex achieves nothing, if it is about to, or has just
changed, then that's just the race condition, the
ioctl would see the value either before, or after,
that change, the mutex doesn't help you know
which, so it doesn't really achieve anything).

If the mutex were useful, you'e used the wrong one,
FSS_ACTIVE is set/cleared under the control of
sc_lock not sc_slock (which is kind of strange as
other flags in the same word are controlled by sc_slock
which does not look like it would be reliable to me).

What I think I'd do is change the code for FSSIOCSET
to be something like

        case FSSIOCSET:
	error = 0;
                if ((flag & FWRITE) == 0)
                        error = EPERM;
                else if ((sc->sc_flags & FSS_ACTIVE) != 0)
                        error = EBUSY;
                if (error == 0) {
	        mutex_enter(&sc->sc_lock);
                        error = fss_create_snapshot(sc, fss, l);
                        if (error == 0)
                                sc->sc_uflags = fss->fss_flags;
                        mutex_exit(&sc->sc_lock); 
	}
                break;

(apologies for any indentation screwups, it looks OK
as I am typing it, but that might not be what it looks like
to anyone else...)

With that you have more the test & set operation, which
should not block if the device is active already.

You could also re-order the two initial tests, so you could
do a read-only open, attempt this operation, which would
always fail then, but with errno==EBUSY if the device is
active, and errno==EPERM otherwise - which would provice
the (racy) just see if it is available operation.

But you need advice from someone who unserstands the locking
issues, and can see if they are used properly with this code
now, and what is really needed to get what you want - don't
just believe this because it looks right to me.

kre