questions about inval_cache_and_wait_for_operation

Aaron Nabil krellboy at gmail.com
Tue Oct 5 05:15:37 EDT 2010


I apologize in advance for my newbie-ness and potential lack of
critical missing information, please correct me off-list and I will do
better.

I'm trying to fix a lock-up problem I'm seeing with jffs2, we run the
system with a cpu-hog for 8 hrs and somewhere between the GC thread
and sync the filesystems lock up.  I can supply more details about
this but I wanted to have all of my ducks in a row first.

While looking for the hang I ran across some code in
inval_cache_and_wait_for_operation I don't understand, and since it's
very close to the problem code I thought I'd ask.


First question....



        unsigned int timeo, sleep_time, reset_timeo;

( . . .)

        sleep_time = chip_op_time / 2;

        for (;;) {

( . . .)

                /* OK Still waiting. Drop the lock, wait a while and retry. */
                spin_unlock(chip->mutex);
                if (sleep_time >= 1000000/HZ) {
                        /*
                         * Half of the normal delay still remaining
                         * can be performed with a sleeping delay instead
                         * of busy waiting.
                         */
                        msleep(sleep_time/1000);
                        timeo -= sleep_time;
                        sleep_time = 1000000/HZ;
                } else {
                        udelay(1);
                        cond_resched();
                        timeo--;
                }

( . . . )
         }


If the execution path EVER takes the (sleep_time >= 1000000/HZ)
branch, it will ALWAYS take that branch every iteration thru the for
loop because the last statement sleep_time = 1000000/HZ sets
sleep_time explicitly to the value which is being tested for as
greater than OR EQUAL to.  Sleep_time is not otherwise changed
elsewhere in the loop.



Second question....

        unsigned int timeo, sleep_time, reset_timeo;

( . . . )

        timeo = chip_op_time_max;
        if (!timeo)
                timeo = 500000;

        sleep_time = chip_op_time / 2;

( . . .)


        for (;;) {

( . . . )

                if (!timeo) {
            // test if timeo is exactly 0
                        map_write(map, CMD(0x70), cmd_adr);
                        chip->state = FL_STATUS;
                        return -ETIME;
                }

                /* OK Still waiting. Drop the lock, wait a while and retry. */
                spin_unlock(chip->mutex);
                if (sleep_time >= 1000000/HZ) {
                        /*
                         * Half of the normal delay still remaining
                         * can be performed with a sleeping delay instead
                         * of busy waiting.
                         */
                        msleep(sleep_time/1000);
                        timeo -= sleep_time;
     //  timeo can underflow
                        sleep_time = 1000000/HZ;
                } else {
                        udelay(1);
                        cond_resched();
                        timeo--;
          // timeo will always hit 0 before underflow
                }

( . . .)


As timeo is an unsigned int and the timeo test is for exactly 0, the
timeo -= sleep_time can underflow and wrap timeo around. If this code
branch was executed only once and sleep_time < timeo this won't be a
problem, but, see first problem above.


Thanks in advance for extending me newbie latitude.


Aaron



More information about the linux-mtd mailing list