search

libnxz / power-gzip

POWER NX zlib compliant library
23 stars 18 forks source link

Increase source_sz when resuming decompression after ERR_NX_DATA_LENGTH #53

Closed tuliom closed 3 years ago

tuliom commented 3 years ago

The NX GZIP user manual predicts a scenario of insufficient source where cc=ERR_NX_DATA_LENGTH, sfbt=0xe and subc >= 0. This patch fixes a corner case of this scenario when the accelerator is not able to progress because it has insufficient source and source_sz is not updated. When that happens, jobs are submitted to the accelerator repeatedly until loop_cnt reaches loop_max when Z_STREAM_END is returned despite the lack of progress, causing an application to wrongly conclude the inflate was complete.

In order to prevent this error from happening, 2 changes are made:

  1. Z_STREAM_ERROR is returned when loop_max is reached because an application has to abort in that case.
  2. Set the last compression ratio to the maximum value in order to increase source_sz to its maximum value when the accelerator returns insufficient source.
abalib commented 3 years ago

Looks good to me. Z_STREAM_ERROR looks right.

Sftb=0xe and subc > 0 means there is partial data in the input buffer that cannot be decompressed, i.e. ask caller to give some more input.

I am looking at zlib proper. If we need more input we need to return Z_BUF_ERROR if I understood zlib.h correctly. https://github.com/madler/zlib/blob/master/zlib.h#L512.

Is it the case that last_compression_ratio is truncating source_sz therefore preventing more input?

tuliom commented 3 years ago

I am looking at zlib proper. If we need more input we need to return Z_BUF_ERROR if I understood zlib.h correctly. https://github.com/madler/zlib/blob/master/zlib.h#L512.

@abalib I don't think this is the case here, though. It's all in the libnxz realm.

Is it the case that last_compression_ratio is truncating source_sz therefore preventing more input?

No. In the scenario I found both source_sz and tpbc were zero, meaning that last_comp_ratio remained unchanged for the next iteration even though NX had insufficient source.

I'm not sure if it's a bad idea to go full on maximum compression ratio (1000UL). I haven't seen any negative impacts yet. We could try to increase it slowly, but there is a chance we would have to create many requests until the required compression ratio is met.

abalib commented 3 years ago

I am good with your changes