[Bug report] Music player stops playing music from sd after some time

Search This thread

notabenem

Senior Member
Jul 17, 2009
118
75

I have just applied the patches from post #833 (thanks zeitferne). Built the kernel (actually the whole ROM, because I don't know how to build specifically the kernel inside omni), flashed and running it. The FPBug test is running for a couple of minutes now, so far so good.
I can upload the CWM/TWRP flashable kernel, if anyone is interested. I also took the liberty of creating a pull-request: https://github.com/omnirom/android_kernel_samsung_smdk4412/pull/3
 
  • Like
Reactions: Lanchon

zeitferne

Senior Member
Jul 15, 2014
153
684
Upper Austria
oberon00.github.io
I have just applied the patches from post #833 (thanks zeitferne). Built the kernel (actually the whole ROM, because I don't know how to build specifically the kernel inside omni), flashed and running it. The FPBug test is running for a couple of minutes now, so far so good.
I can upload the CWM/TWRP flashable kernel, if anyone is interested. I also took the liberty of creating a pull-request: https://github.com/omnirom/android_kernel_samsung_smdk4412/pull/3

If OmiROM handles that like CyanogenMod, they will not use GitHub pull request and you should instead submit the patches to gerrit.omnirom.org.
 
  • Like
Reactions: Lanchon

notabenem

Senior Member
Jul 17, 2009
118
75
Right! I forgot about your post, so there's no need to upload my build, because it will essentially be the same. Thanks for the reminder. What I will do though, is submit those patches to Omni via gerrit (unless someone, Entropy perhaps, will do it faster).

Update:
Struggling with gerrit. Some of the patch messages are missing a comit id in the footer and gerrit refuses them:
Code:
[remote rejected] HEAD -> refs/for/android-4.4 (missing Change-Id in commit message footer)
Any suggestions, how to submit?
 
Last edited:

notabenem

Senior Member
Jul 17, 2009
118
75
Another thing: in the kernel I built, I can't turn on the wifi, even though the modules were part of the zip and were flashed by TWRP. dhd.ko is inside /system/lib/modules/ with the correct size ...
 

Lanchon

Senior Member
Jun 19, 2011
2,751
4,487
Right! I forgot about your post, so there's no need to upload my build, because it will essentially be the same. Thanks for the reminder. What I will do though, is submit those patches to Omni via gerrit (unless someone, Entropy perhaps, will do it faster).

Update:
Struggling with gerrit. Some of the patch messages are missing a comit id in the footer and gerrit refuses them:
Code:
[remote rejected] HEAD -> refs/for/android-4.4 (missing Change-Id in commit message footer)
Any suggestions, how to submit?

make a squashed single commit. i think changes are supposed to be a single commit anyway?

---------- Post added at 09:36 PM ---------- Previous post was at 09:33 PM ----------

Another thing: in the kernel I built, I can't turn on the wifi, even though the modules were part of the zip and were flashed by TWRP. dhd.ko is inside /system/lib/modules/ with the correct size ...

dont worry, it must be your build process, not the patches. (you can test by building without fpbug patch and checking that wifi fails too.) omni's build bot will work fine.
 

notabenem

Senior Member
Jul 17, 2009
118
75
make a squashed single commit. i think changes are supposed to be a single commit anyway?

---------- Post added at 09:36 PM ---------- Previous post was at 09:33 PM ----------



dont worry, it must be your build process, not the patches. (you can test by building without fpbug patch and checking that wifi fails too.) omni's build bot will work fine.

Apologies, I did not want to create the impression, that the patches caused the bug. Before posting the previous message I did exactly as you suggested here and the build from the original repo had the same problem with the wifi, so it's clear that I am doing something wrong, just can't figure out what...
 
  • Like
Reactions: Lanchon

FrodgE

Senior Member
Aug 27, 2010
85
62
Melbourne
Right! I forgot about your post, so there's no need to upload my build, because it will essentially be the same.
Hehe thought you might have ! I haven't looked too closely but I think the only recent Omni kernel changes are specific to the S3. Still no reason why you shouldn't go through the steps as personal learning experience, that's half the reason why I did it ! For gold on how to compile just the kernel check out master Lanchon's comments near my kernel build post (I had the exact same questions).

Before posting the previous message I did exactly as you suggested here and the build from the original repo had the same problem with the wifi, so it's clear that I am doing something wrong, just can't figure out what...
Are you trying to flash just a kernel zip over the regular nightlys here or do you also get this problem if you flash your entire "homebuilt" ROM ? Is it possible you've just messed up the zip file ? Maybe you can pinch mine (which I pinched from someone else) and try modifying it to suit ? Do the md5 sums check out as well as just the size ? Can permission issues cause this ?
 

notabenem

Senior Member
Jul 17, 2009
118
75
Hehe thought you might have ! I haven't looked too closely but I think the only recent Omni kernel changes are specific to the S3. Still no reason why you shouldn't go through the steps as personal learning experience, that's half the reason why I did it ! For gold on how to compile just the kernel check out master Lanchon's comments near my kernel build post (I had the exact same questions).


Are you trying to flash just a kernel zip over the regular nightlys here or do you also get this problem if you flash your entire "homebuilt" ROM ? Is it possible you've just messed up the zip file ? Maybe you can pinch mine (which I pinched from someone else) and try modifying it to suit ? Do the md5 sums check out as well as just the size ? Can permission issues cause this ?

Thanks for the valuable advice.
I only flashed the kernel, not the complete rom - although I may just try to flash it.
I checked my zip against your zip of course. Included all the modules, made sure they were flashed. Also tried the "fix permission" exercise, but so far no cigar. I will have to check the MD5s (or binary comparison)
 

Lanchon

Senior Member
Jun 19, 2011
2,751
4,487
Thanks for the valuable advice.
I only flashed the kernel, not the complete rom - although I may just try to flash it.
I checked my zip against your zip of course. Included all the modules, made sure they were flashed. Also tried the "fix permission" exercise, but so far no cigar. I will have to check the MD5s (or binary comparison)

did you pull the blobs from a github repo?

https://github.com/DonkeyCoyote
https://github.com/ThankYouMario
https://github.com/TheMuppets
https://github.com/PupPeople
etc...

i think DonkeyCoyote is used for omni.
 

Entropy512

Senior Recognized Developer
Aug 31, 2007
14,088
25,086
Owego, NY
make a squashed single commit. i think changes are supposed to be a single commit anyway?
Actually in this particular case, since most of the commits are upstream commits, it is preferred not to squash them.

Finally managed to submit the patch(es) to Omni's gerrit: https://gerrit.omnirom.org/11614
Can you try and resubmit as a separate set of commits?

You might need to do something like:
Code:
git rebase -i omnirom/android4.4
and choose the "reword" option to cause git to redo the commit message for each commit. This should, when you save the "changed" commit message (even if you changed nothing), cause it to add a changeID
 

notabenem

Senior Member
Jul 17, 2009
118
75
Actually in this particular case, since most of the commits are upstream commits, it is preferred not to squash them.
In that case please cancel this change and instead I will submit 12 separate changes.
Alternatively, I can do 1 request for all the upstream changes (still squashed) and 1 separate for zeitferne's changes.
 

Lanchon

Senior Member
Jun 19, 2011
2,751
4,487
In that case please cancel this change and instead I will submit 12 separate changes.
Alternatively, I can do 1 request for all the upstream changes (still squashed) and 1 separate for zeitferne's changes.

i think he wants those as separate commits. and i think you can abandon your own change yourself in gerrit.
 
  • Like
Reactions: notabenem

GidiK

Senior Member
Jan 8, 2013
113
103
Best
Gerrit is quite new to me, apologies for the newbie mistakes. I have submitted 13 separate changes (one for each upstream change, and one for zeitferne's changes).
These are from https://gerrit.omnirom.org/#/c/11621/ to https://gerrit.omnirom.org/#/c/11633/

I have made a build (omni-4.4, i9100) with all 13 patches applied. I reconfirmed that I could reproduce the fuse bug with a recent nightly and ran the exact same test with the new build. And good news: the bug is gone! So as far as I am concerned, they are good to go.

The test I did:

Code:
adb shell
cd /storage/sdcard1
tar c Movies > /dev/null

while in another terminal window monitor progress with:

Code:
adb shell
iostat -k 5

On the recent nightly it would get stuck the moment the screen went off.

Thanks to all involved in finding this solution!
 

Top Liked Posts

  • There are no posts matching your filters.
  • 55
    CM11-M10 Music Bug Fix

    this is just sdcard.c compiled with "-mfloat-abi=soft" instead of "-mfloat-abi=softfp". disassembly confirms that regular instead of FP registers are used to temporarily save unique.

    sdcard.c is usually left alone in roms. so although this fix is for CM11-M10, it probably works fine on all kitkat roms.
    @GidiK, could you please test? just flash from recovery.
    thank you!
    42
    thanks guys, this is great!! pheww... that was a *****

    i'll clean up and post everything later cause i'm busy now, but here's what did it:
    on SMP arm (eg: our case, multicore), the FPU state is saved eagerly on context switch out and restored lazily on context switch in. a given time slice starts with the FPU disabled, and only if the process later touches the FPU during the slice, the kernel restores the state in a trap (and only then it will have to save the state when the slice is up).

    the state is saved in ram but also left there in the disabled FPU registers. it may happen that when the kernel decides to restore the FPU state saved in ram to a given disabled FPU, by chance the state it wants to load is already present in there. to take advantage of this lucky situation, the kernel tracks the leftover state in the disabled FPUs and optimizes the load away when it can prove that it's not needed. (the trap is still needed to enable the FPU, so the time saved is really not that much.)

    in our case somehow the tracking fails (across power management state changes, or during CPU migration of tasks, or who knows), the wrong decision is made and the state is not restored when it's actually needed. FR has a bunch of mainline fixes for FPU corruption cases (same as MG8), but the change that did away with this problem is simply disabling this state restore optimization and always load the state from ram.

    this looks like a full fix. it does have a performance impact, but only for processes that use the FPU, and only when they are running FPU-uncontested in a core. in any case, i'd guess the performance impact of affected processes is probably less than 1%.


    now i'd really like someone with a 4412 device (eg: S3 international) to properly test FPBug2.apk (screen off) and verify that it's truly not affected.

    ---------- Post added at 02:22 PM ---------- Previous post was at 02:16 PM ----------

    I must be doing something wrong. Flashed the kernel, but I am stuck at the boot animations screen (rest of the system is still Omni, no data-wipe)

    this is a CM kernel, no reason it should work in Omni

    This only patches the sdcard daemon, and thus doesn't affect testing with Lanchon's FPBug App, but it would of course affect tests that depend on playing music/copying files.

    that's right!

    ---------- Post added at 02:28 PM ---------- Previous post was at 02:22 PM ----------

    i'm curious as to the performance impact, see if it's worth the effort to restore the optimization.

    if somebody wants to test:
    after reboot please run antutu 2 or 3 times with each kernel (FR and MG8) and report
    thanks!!!
    36
    If you want to have a workaround until this bug is properly fixed, try the attached flashable zip.

    Warning: tested on i9100 variant only.
    Warning2: since we are dealing with an obscure memory/variable corruption bug, it is possible that something else goes wrong with the workaround.
    To go back to original, flash your rom zip again (it will overwrite the sdcard binary)

    For me, this workaround fixes any issues and the file copy testcase I have used to debug this bug has now been passing several times and across reboots

    What this workaround does:
    Code:
    $ git diff sdcard.c
    diff --git a/sdcard/sdcard.c b/sdcard/sdcard.c
    index 989ca00..67e5910 100644
    --- a/sdcard/sdcard.c
    +++ b/sdcard/sdcard.c
    @@ -1214,9 +1214,10 @@ static int handle_read(struct fuse* fuse, struct fuse_handler* handler,
             const struct fuse_in_header* hdr, const struct fuse_read_in* req)
     {
         struct handle *h = id_to_ptr(req->fh);
    -    __u64 unique = hdr->unique;
    +    volatile __u64 vars64[2];
    +    vars64[0] = hdr->unique;
         __u32 size = req->size;
    -    __u64 offset = req->offset;
    +    vars64[1] = req->offset;
         int res;
     
         /* Don't access any other fields of hdr or req beyond this point, the read buffer
    @@ -1224,15 +1225,15 @@ static int handle_read(struct fuse* fuse, struct fuse_handler* handler,
          * saves us 128KB per request handler thread at the cost of this scary comment. */
     
         TRACE("[%d] READ %p(%d) %u@%llu\n", handler->token,
    -            h, h->fd, size, offset);
    +            h, h->fd, size, vars64[1]);
         if (size > sizeof(handler->read_buffer)) {
             return -EINVAL;
         }
    -    res = pread64(h->fd, handler->read_buffer, size, offset);
    +    res = pread64(h->fd, handler->read_buffer, size, vars64[1]);
         if (res < 0) {
             return -errno;
         }
    -    fuse_reply(fuse, unique, handler->read_buffer, res);
    +    fuse_reply(fuse, vars64[0], handler->read_buffer, res);
         return NO_STATUS;
     }

    update: made the update script a bit safer
    35
    Alternative fix, one level deeper

    After my investigations in my post above, I just had to put the pieces together:
    in our case somehow the tracking fails (across power management state changes, or during CPU migration of tasks, or who knows)

    I based my kernel on @Lanchon's (this one), removed the last commit that always reloads the FPU and also merged three other commits related to CPU_PM. Here is the gist with the patches:
    https://gist.github.com/9fb17c20e635bbffcb7f

    I hope the corruption does not come back after a second restart ;) but so far the FPBug app displayed nothing.

    I don't know if these low power states are supposed to corrupt the FPU registers, but if they are, this fix should actually adress the root cause.

    EDIT: I have added a flashable ZIP with this kernel (MD5: 55b591535854b66ef0de3245183eb33e).
    EDIT2: Updated the ZIP to include driver modules, so that WiFi now works. New MD5: 4d9cd2b9021a4652f7d14ea5a101291d.
    EDIT3: Fixed a potential bug. Since the kernel was running fine for (almost?) everyone, users don't need to update, but kernel developers should take a look at the change: https://gist.github.com/Oberon00/9f...xynos-call-pm-notifiers-w-irqs-disabled-patch. New MD5: 4852de8e7c3b77878290a72519b8004d

    EDIT4: Added a flashable ZIP for Note1/N7000. MD5: df4fb01a03f10ac88309616cddc787ce. WARNING: As I do not own this phone, I cannot test the kernel or the flash-procedure there. Better make a backup before you flash it!

    For a full patchset (includes Lanchon's required changes), see here: http://xdaforums.com/showpost.php?p=57654708&postcount=833.